[FFmpeg-devel] [PATCH 1/2] avcodec/s302m: enable non-PCM decoding

[Kieran Kunhya]

On Thu, 15 Feb 2024 at 16:48, Gyan Doshi <ffmpeg at gyani.pro> wrote:

>
>
> On 2024-02-15 09:40 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2024-02-15 13:31:59)
> >> On 2024-02-15 04:17 pm, Anton Khirnov wrote:
> >>> Hi,
> >>> sorry for the delay, I've been busy fixing things for the release
> >>> Quoting Gyan Doshi via ffmpeg-devel (2024-01-29 05:00:33)
> >>>> On 2024-01-28 04:24 pm, Anton Khirnov wrote:
> >>>>>> a) it would mean essentially inlining this decoder in the demuxer.
> >>>>> Why is that a problem? This decoder seems like it shouldn't be a
> >>>>> decoder.
> >>>>>
> >>>>> I agree with Andreas that this seems like it's a demuxer pretending
> to
> >>>>> be a decoder.
> >>>> This module transforms the entire raw payload data to generate its
> >>>> output, even if the syntax is simple which
> >>>> essentially makes it a de-coder. The de-multiplexer aspect of multiple
> >>>> streams is an academic possibility allowed
> >>>> by the standard but not seen in any sample which makes me suspect it's
> >>>> used for carriage between broadcast
> >>>> facilities rather than something ever sent to an OTT provider, let
> alone
> >>>> an end user.
> >>> If it dynamically generates nested decoders, then it's not a proper
> >>> codec in our model. It should be either a part of the demuxer, or a
> >>> bitstream filter (possibly inserted automatically by the demuxer).
> >> s302m is a hybrid creature and does not slot cleanly into any role. So
> >> there is no theoretically proper place for this component - any choice
> >> is a least-out-of-place accommodation.
> >>
> >> But it is much more out of place inside a demuxer. Analyzing packet
> >> payload and then manipulating that entire payload is much closer to a
> >> decoding role than data chunk extraction for packetization.
> > I don't see why specifically this property should be the one
> > distinguishing demuxers from decoders, it sounds pretty arbitrary to me.
> > Many demuxers apply transformations of far higher complexity to the
> > bytestream before exporting it, e.g. in matroska the packet data may be
> > compressed, laced, etc.
> >
> >> And the stream extracted from the container is meant to be SMPTE ST
> >> 302 not PCM* or Dolby-E/AC-3..etc,
> > "meant to be"? By whom?
> >
> > The point of libavformat is to abstract away the differences between
> > containers as much as is reasonably feasible, and export the data in the
> > format most useful to the caller for decoding or other processing.
> >
> >> which will both misrepresent what the container carries
> > Why should the caller care?
> >
> >> and possibly discard S-ADM metadata, if present, in the packet.
> > Why could that not be exported as side data?
> >
> >> A bsf in principle would work but in practice, can't as Andreas
> >> clarified that bsfs can't set or alter codec_id after init. And
> >> resetting the codec id requires packet inspection.
> > There are two possibilities then - either extend the BSF API to support
> > multiple output streams, or implement it inside libavformat as a
> > post-demuxer hook called in the same place as parsing.
> >
> >> Nested decoders are used without issue in components like imm5 or ftr
> >> (upto 64 nested decoders!) among others. There's no breaking of new
> >> ground here.
> > Nested decoders are certainly not "without issue" - they are a constant
> > source of issues, since implementing nesting properly is very tricky. I
> > am fairly sure most nested decoders we have are subtly broken in various
> > ways.
>
> This patch facilitates a certain productive use of ffmpeg with respect
> to processing of live inputs that wasn't possible earlier,
> and which currently is being used successfully by multiple people over
> the past few weeks.
> It applies a processing model already implemented in multiple other
> decoders for a number of years. I haven't seen many reports
> of issues with them. And surely something being 'a constant source of
> issues' would be a lot more than 'subtly broken' as you describe
> them.You're the only one who has objected on architectural grounds and
> this looks to be a fundamental disagreement.
> If you are blocking this patch, do acknowledge here within 24 hours and
> we can send this to the TC else I'll push it after that period.
>

Dolby E can exist in other containers apart from S302M. Raising the TC is
premature.

Kieran