[FFmpeg-devel] [PATCH] libavcodec/mmaldec.c: add interlaced_frame and format struct to AVFrame for deinterlacing]

Mark Thompson sw at jkqxz.net
Sun Jul 31 18:33:00 EEST 2016


On 31/07/16 15:51, Jens Ziller wrote:
> Am Samstag, den 30.07.2016, 17:30 +0100 schrieb Mark Thompson:
>> Does this also do the right thing if the decoder is reconfigured with
>> frames outstanding?  To me (without really knowing the code) it looks
>> like it will overwrite the old format (line 702) and therefore mess
>> with existing frames, though there are multiple levels of indirection
>> so the frame could be holding onto a reference by some route I'm not
>> seeing.
> MMAL_EVENT_FORMAT_CHANGED is set before the first decoded frame give
> out. I have never seen that this flag is set twice between start/stop
> decoder.

Try the test stream h264/reinit-large_420_8-to-small_420_8.h264 in FATE?

>> Similarly for stopping the decoder - there may be output frames which
>> are still valid, and it looks to me like the format structure will
>> have disappeared.  (Is there some internal reference via the
>> MMAL_BUFFER_HEADER_T which solves both of these cases, maybe?  If so,
>> it might be clearer if you could use that internal reference to set
>> the value rather than going via the decoder.)
> The struct MMAL_ES_FORMAT_T will copied in the MMAL_PORT_T struct to
> configure the component. There it lives until the application change
> anything.

Hmm, that's not quite what I meant to ask; apologies because the question wasn't very clear.

Consider this sequence, where we want to decode a single image and then do something with it:

open decoder
decode frame
close decoder
open <some other component>
give it the frame we got from the decoder

To me that looks like it will invoke undefined behaviour (segfault or read garbage) when trying to access data[2] in the second component, because the pointer appears to be to the MMAL_ES_FORMAT_T in the MMAL_PORT_T on the decoder which we just destroyed.  If not, where is the reference which keeps that pointer valid living?

>> Also, will this structure always be available with sufficient
>> lifetime for MMAL frames produced by things other than the
>> decoder?  Your documentation on the pixfmt is phrased such that it is
>> always required - given that it appears to be for a specific use-
>> case, maybe it would be better if it were optional.
> Pixelformat AV_PIX_FMT_MMAL is a MMAL specific format. It makes only
> sense to use it with MMAL components. All MMAL components needs a
> MMAL_ES_FORMAT_T. The MMAL documented and easiest way is to copy the
> struct from decoder to the components.

Right, that makes sense.  Thank you for clarifying :)

- Mark



More information about the ffmpeg-devel mailing list