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

[wm4]

On Wed, 20 Nov 2013 20:25:20 +0100
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).
> 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.

Uh, what? Wouldn't these AVI files then have ffmpeg specific side data
inside of the packet data? Sounds 100% broken and non-portable.

> 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.

Side data seems like a nice way to communicate parameter changes and
other out-of-band data to the decoder side. How else would you handle
parameter changes?