[FFmpeg-devel] [PATCH 1/4] lavf: add write_uncoded_frame() API.

Nicolas George george at nsup.org
Sat Jan 4 10:45:15 CET 2014


Le tridi 13 nivôse, an CCXXII, Stefano Sabatini a écrit :
> You have a format context, a stream index, now you want to know if
> stream supports uncoded frames. So you do:
> 
> ret = av_write_uncoded_frame(s, stream_index, NULL, QUERY);
> 
> and check the result.
> 
> So you are basically using the av_write_uncoded_frame() function to
> check functionality of the muxer. This rings a bell in my head since
> you are basically overloading the write frame function with a query
> feature.
> 
> This may be bad in some cases. Suppose for example you have a complex
> logic to set what is accepted by the muxer, if you have the query code
> in the frame writing function you call it for every frame.
> 
> Probably a better way would be to do something like:
> ret = avformat_support_uncoded_frame_muxing(s, stream_index);
> 
> Note that this is not a strong objection (for example you could lazily
> evaluate the check only on the first call) but I feel like there is
> probably a cleaner way to implement this.

I do not have a strong opinion about this either. One reason I can see to
use the QUERY flag instead of a separate function is this:

The generic muxing code and each muxer will always perform basic consistency
checks, to avoid working with unsupported data and crashing randomly. These
checks are cheap compared to the actual muxing work, and mostly they amount
to what is necessary for the QUERY code.

Therefore, implementing the QUERY code amounts to adding a single line:

	if ((flags & QUERY)) return 0;

just between the consistency checks and the actual work.

On the other hand, having av_write_uncoded_frame_supported() separate from
av_write_uncoded_frame() means two different code paths, with a non-zero
probability for the checks getting out of sync at some point.

Note that it is possible to make the QUERY flag internal to mux.c and just
add:

int av_write_uncoded_frame_supported(AVFormatContext *ctx, int stream_index)
{
    return !av_write_uncoded_frame(ctx, stream_index, NULL,
                                   AV_WRITE_UNCODED_FRAME_FLAG_QUERY);
}

so that the code paths are the same but the application API is different.

Regards,

-- 
  Nicolas George
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140104/5bb8c931/attachment.asc>


More information about the ffmpeg-devel mailing list