[FFmpeg-devel] [PATCH] [RFC] avformat/options_table: do not merge sidedata by default

[Reimar Döffinger]

On 20.11.2013, at 15:59, wm4 <nfxjfg at googlemail.com> wrote:
> On Wed, 20 Nov 2013 00:42:38 +0100
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
>> This requires applications to pass side data from demuxer to
>> decoder and demuxer to muxer.
>> If we want this change then a #if MAJOR_VERSION would need to be
>> added around it as it adds the new requirement of passing side data
>> around too (through all queues an application has between its demuxer
>> and muxer/decoder layers)
>> 
>> Do applications support this kind of side data passing already ?
>> 
>> Suggested-by: wm4
>> Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
>> ---
>> libavformat/options_table.h |    2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/libavformat/options_table.h b/libavformat/options_table.h
>> index 8145325..76b9d6a 100644
>> --- a/libavformat/options_table.h
>> +++ b/libavformat/options_table.h
>> @@ -37,7 +37,7 @@ static const AVOption avformat_options[] = {
>> {"direct", "reduce buffering", 0, AV_OPT_TYPE_CONST, {.i64 = AVIO_FLAG_DIRECT }, INT_MIN, INT_MAX, D|E, "avioflags"},
>> {"probesize", "set probing size", OFFSET(probesize), AV_OPT_TYPE_INT, {.i64 = 5000000 }, 32, INT_MAX, D},
>> {"packetsize", "set packet size", OFFSET(packet_size), AV_OPT_TYPE_INT, {.i64 = DEFAULT }, 0, INT_MAX, E},
>> -{"fflags", NULL, OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = AVFMT_FLAG_FLUSH_PACKETS }, INT_MIN, INT_MAX, D|E, "fflags"},
>> +{"fflags", NULL, OFFSET(flags), AV_OPT_TYPE_FLAGS, {.i64 = AVFMT_FLAG_FLUSH_PACKETS | AVFMT_FLAG_KEEP_SIDE_DATA}, INT_MIN, INT_MAX, D|E, "fflags"},
>> {"flush_packets", "reduce the latency by flushing out packets immediately", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_FLUSH_PACKETS }, INT_MIN, INT_MAX, D, "fflags"},
>> {"ignidx", "ignore index", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_IGNIDX }, INT_MIN, INT_MAX, D, "fflags"},
>> {"genpts", "generate pts", 0, AV_OPT_TYPE_CONST, {.i64 = AVFMT_FLAG_GENPTS }, INT_MIN, INT_MAX, D, "fflags"},
> 
> Background: if this flag is unset, libavformat appends side data to the
> packet data, using a magic 64 bit constant as footer (see
> av_packet_merge_side_data). libavcodec calls av_packet_split_side_data(),
> which unpacks the side data if it recognizes the magic footer.
> 
> From an application point of view, this appends random data to the
> demuxed packet data (which us bad), and also clears any side data from
> the packet (which is bad).
> 
> Also, should raw data happen to contain the footer, libavcodec might
> interpret it as side data, which is kind of inelegant to say the least.
> 
> Libav doesn't have this mechanism, so any application which actually
> wants to work on a wide range of distributions has to support passing
> through side data. Likewise, applications primarily developed on Libav
> might run into trouble if they don't see the side data, or if they
> don't like random data appended to demuxed packets.
> 
> I'd say libavcodec shouldn't attempt to split side data by default
> either. A flag should be added for it, and it should be off by default.
> Applications which for some reason don't want to pass along side data
> (MPlayer?) can enable this hack explicitly.

Without this feature you cannot remux certain files into e.g. AVI or other containers without support for sidedata correctly (for some values of correct I admit).
So this will change behaviour of ffmpeg in a quite major way, libavformat itself has no way to correctly decide if it is e.g. safe to just discard the side data.
I guess the best I can say is this sidedata thing was a braindead idea from the start, feel free to twiddle with flags (I'll try to remember to just counterfiddle on MPlayer side) but I have some doubts it will do more than annoy people. If you want it to actually work you'll need to invest a lot more time.