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

[Michael Niedermayer]

Hi Kieran

On Fri, Dec 13, 2024 at 09:24:17AM +0000, Kieran Kunhya via ffmpeg-devel wrote:
> On Fri, 13 Dec 2024, 02:46 Michael Niedermayer, <michael at niedermayer.cc>
> wrote:
> 
> > Hi
> >
> > On Fri, Dec 13, 2024 at 01:40:43AM +0000, Kieran Kunhya via ffmpeg-devel
> > wrote:
> > > On Thu, 12 Dec 2024, 13:07 Nicolas Gaullier, <nicolas.gaullier at cji.paris
> > >
> > > wrote:
> > >
> > > > >De : ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> de la part de
> > Anton
> > > > Khirnov <anton at khirnov.net>
> > > > >Envoyé : jeudi 12 décembre 2024 09:36
> > > >  >
> > > > >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.
> > > > >
> > > >
> > > > Thank you Anton for taking time to precise your mind.
> > > > I understand your point.
> > > > Unfortunately, I don't know how to move on.
> > > > I think I will shelve this work until someone has an idea how to deal
> > with
> > > > it.
> > > >
> > >
> > > It is required in his patchset because you could have PCM at 48khz, then
> > > Dolby E at some other sample rate that's expected to be output at 48khz.
> > > Any of these can change on the fly but the base rate is 48khz (in order
> > to
> > > have a simple frame cadence with NTSC video)
> >
> > At the risk of steping on some specs and some expectations.
> >
> > what would happen if the decoder simply outputs what is stored ?
> > I mean if a packet on the input that encodes 1234 samples at 48khz
> > it outputs 1234 samples at 48khz. If the next packet contains
> > 7654 samples at 23khz then it outputs a packet of 7654 samples at 23khz
> >
> 
> Does FFmpeg even support a new sample rate per AVFrame?

AVFrame has a sample_rate field and a timestamp, so yes

for actual resampling, in swresample. The resampler can be closed
and reopened with a new sampling rate OR

swresample can smoothly adjust for sample rate drift based on only the
timestamps.

Thats whats supported today.

To support something else, the code would need to be slightly adjusted
I dont know what dolby exactly expects to be done with the samples around
the transition. But whatever it is it can be implemented if its not supported


> 
> The weird coded sample rates are chosen specifically for conversion to
> 48khz and the matching video framerate. There's metadata that needs to
> remain associated with that audio/video frame that would otherwise get
> destroyed if the sample rate conversion and framing was downstream.

This sounds not specific to this. I mean any metadata associated with a
frame should continue to be associated with that range of samples
through the filter chain

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

During times of universal deceit, telling the truth becomes a
revolutionary act. -- George Orwell
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20241213/192ab7b8/attachment.sig>