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

[Hendrik Leppkes]

On Wed, Nov 20, 2013 at 8:25 PM, Reimar Döffinger
<Reimar.Doeffinger at gmx.de> wrote:
> 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).

This is just terrible, in fact, its one of the worst things i've read
here for a while.
If a codec needs out-of-band data, and the container does not support
it, those two should simply not go together, and FFmpeg should not
allow their creation.

We complain all the time when a commercial software invents a new hack
to store a random codec into a container format that it was not
designed for, and invent new proprietary ways to screw with data.
And you want FFmpeg to do the same? For shame.

Muxers should produce compliant files, or not produce them at all.
Anything else is just plain and simple, wrong.

- Hendrik