[FFmpeg-devel] FFmpeg TC input needed

[Paul B Mahol]

How dare you to question LibAV overlords decisions here!

On Mon, Apr 8, 2024 at 3:12 PM Gyan Doshi <ffmpeg at gyani.pro> wrote:

> Ping x2.
>
> On 2024-04-02 10:55 am, Gyan Doshi wrote:
> > Ping.
> >
> > As the TC rules matter has been concluded, this should go ahead.
> >
> > Regards,
> > Gyan
> >
> >
> > On 2024-02-17 05:15 pm, Gyan Doshi wrote:
> >> Issue:
> >>
> >> Patch: avcodec/s302m: enable non-PCM decoding
> >> URL:
> >>
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240127103854.9971-1-ffmpeg@gyani.pro/
> >>
> >> The issue needing resolution is whether the patch should be added to
> >> the existing s302m decoder or should that decoder
> >> be removed and all old and new patched features inlined into the
> >> mpeg-ts demuxer.
> >>
> >> Summary:
> >>
> >> SMPTE ST 302 specifies for carriage of LPCM media in MPEG-TS. SMPTE
> >> ST 337 enables this for non-PCM codecs. The payload
> >> has a custom layout so it can't be directly processed hence
> >> lavc/s302m decodes the packet data to yield LPCM media. But
> >> it can only deal at present with LPCM payloads, meaning that non-PCM
> >> payloads need to be exported to a raw bytestream
> >> format and then decoded in a 2nd step, which prohibits direct
> >> transcoding of live/streaming inputs. This patch corrects
> >> the identification process for non-PCM payloads, reformats the
> >> payload and then carries out in-place decoding by calling
> >> a nested decoder similar to the ftr or imm5 decoders in lavc.
> >>
> >> In the v1 patch review, Andreas, in response to the proposed doc
> >> entry describing the feature capability of multiple
> >> or differing payloads in a s302m stream, suggested[1] that s302m
> >> should be a bitstream filter instead, but I did not
> >> see that as an actionable suggestion as he immediately listed the bsf
> >> limitations preventing the possibility. I also
> >> had not seen an actual sample of s302m with multiple embedded
> >> streams. Kieran also observed[2] that he had not seen
> >> such a stream in the wild. So the added features of this patch,
> >> wherever they are ultimately located, shall not yield
> >> more than one media stream. Anton suggested[3] that the decoder
> >> should instead be a demuxer. I saw no other objections
> >> to the architecture of the patch.
> >>
> >> I posted the v2 patch, incorporating some changes suggested by
> >> Andreas, 4 days later. This had gone uncommented for
> >> over two weeks when I posted a notice stating an intention to push.
> >> Anton posted[4] a new objection that "If it
> >> dynamically generates nested decoders, then it's not a proper codec
> >> in our model". This new objection is not connected
> >> to multiple streams but only to a codec 'model' that I don't see
> >> described anywhere and which contradicts the
> >> implementations of multiple decoders with a nested decoder, including
> >> the ftr and imm5 decoders, which are most similar
> >> in design to the patched s302m decoder. Anton later on mentioned[5]
> >> that nested decoders are "a constant source of
> >> issues". However, I didn't find anything on trac reporting an issue
> >> with the nested decoders of ftr and imm5 nor
> >> anything on ffmpeg-devel-ml or ffmpeg-user-ml. Nothing in their
> >> commit history either points to architectural bugs.
> >> These decoders have been around for 6 years among themselves. The
> >> testing of the patched s302m decoder over the past
> >> month by myself, an OTT provider and others shows no issues either.
> >> Finally, Anton speculates[6] that the burden of
> >> fixes will likely fall upon him. In none of his objections, till the
> >> time of writing, did I see specific concerns with
> >> the patch.
> >>
> >> There are some limitations in shifting this decoder wholesale to
> >> inside the MPEG-TS demuxer. A s302m stream may contain
> >> some non-media payload accompanying non-PCM media i.e. S-ADM
> >> metadata. At present, I have neither the samples nor the
> >> specification needed in order to locate and extract or parse this
> >> metadata. Formatting the payload data inside the
> >> demuxer will lead to irrevocable loss of such metadata if present.
> >> However, a decoder patch allows simultaneuous output
> >> of both a decoded stream alongside a copied stream. The end-user can
> >> then do with the raw data whatever they wish.
> >>
> >> Ultimately, s302m is specified an an elementary stream inside a
> >> MPEG-TS container. Its internal handling is better left
> >> to a dedicated module like a decoder. A bitstream filter might be a
> >> better fit if s302m streams with multiple media
> >> payloads ever start appearing - none have, so far - but for single
> >> media payloads, a decoder remains the best place.
> >>
> >> Regards,
> >> Gyan
> >>
> >> [1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-January/320119.html
> >> [2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-January/320321.html
> >> [3]: https://ffmpeg.org/pipermail/ffmpeg-devel/2024-January/320258.html
> >> [4]:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321514.html
> >> [5]:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321523.html
> >> [6]:
> https://ffmpeg.org/pipermail/ffmpeg-devel/2024-February/321539.html
> >>
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel at ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>