[FFmpeg-devel] [PATCH 0/7] RFC: complete rework of s337m support

[Anton Khirnov]

Hi Nicolas,
Quoting Nicolas Gaullier (2024-12-06 10:37:03)
> >De : Anton Khirnov <anton at khirnov.net>
> >Envoyé : jeudi 5 décembre 2024 15:06
> >Hi,
> >I believe a similar approach was tried by Gyan earlier this year and
> >unanimously rejected by the TC [1-3].
> >
> >[1] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240127103854.9971-1-ffmpeg@gyani.pro/
> >[2] Message-Id <9be400cf-949f-4eb1-93a1-b352a1b807e6 at gyani.pro>
> >    https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321564.html
> >[3] Message-Id <20240412122920.GB3370 at haasn.xyz>
> >    https://lists.ffmpeg.org/pipermail/ffmpeg-devel/2024-April/325588.html
> 
> Anton,
> Yes, of course, I followed this work.
> I thought I had highlighted many new elements (including many s337m issues in current code),
> and my work is not focused on s302m,
> so I would have expected some discussions to start.
> You certainely noticed that I posted this as an "RFC", so it is more "open".
> 
> To take an example, I will quote one point of the TC:
> "The opposition to the patch was based on the opinion that this implementation of
> S302M should be handled by libavformat, not libavcodec."
> 
> In my patch serie, the s302m decoder IS moved to libavformat, which is what was asked.
> So: even if there are some strong/hard points against the s337m decoder I designed,
> maybe part of this work can be helpful. Maybe just the s302m decoder moved to libavformat,
> I don't know.

I am not intimately familiar with these formats and their mutual
relationship, so these problems you refer to are not clear to me.
What I am familiar with is our APIs, and from that position I can say
this:
1) Nested decoders are huge red flag. For one thing, they are very
   tricky to implement properly - almost every one I've touched had
   subtle bugs that were fixed by moving to a non-nested design.

   But even more importantly it's a sign that there is something fishy
   going on, and maybe the thing you're dealing with is not really a
   codec and should be handled in some other layer. It's more of a rule
   of thumb than a hard rule, but it does mean that nested decoders need
   a strong argument justifying them.
2) In-decoder resampling is another big red flag. We have consciously
   moved a long time ago towards decoders exporting their native format
   and (whenever feasible) not doing any format conversion internally.
   Opus is a special case in that resampling there is required by the
   spec to combine CELT with SILK.

   Again, you could claim that resampling is also fundamentally required
   in your case, but then you should have a strong supporting argument.

-- 
Anton Khirnov