[FFmpeg-devel] FFmpeg TC input needed

[FFmpeg-devel] FFmpeg TC input needed

[Gyan Doshi]

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@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Kieran Kunhya]

On Sat, 17 Feb 2024, 11:46 Gyan Doshi, <ffmpeg@gyani.pro> 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.
>

Additional comments:

Dolby E can exist in any PCM container (wav, MP4). S302M just happens to be
a tricky one.

Whilst s302m multiple substreams I haven't seen, Dolby E streams internally
contain multiple programs, often 5.1 and a 2.0 downmix.

Strictly speaking you have to handle the case where the Dolby E frame is
misaligned and spans two S302M packets which does exist (and there is
ambiguity about the PTS).

There is a 3x3 matrix of flavours between coded Dolby E and the carriage
PCM (16-bit, 20-bit, 24-bit).

Kieran
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Gyan Doshi]

On 2024-02-17 05:31 pm, Kieran Kunhya wrote:
> On Sat, 17 Feb 2024, 11:46 Gyan Doshi,<ffmpeg@gyani.pro>  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.
>>
> Additional comments:
>
> Dolby E can exist in any PCM container (wav, MP4). S302M just happens to be
> a tricky one.
>
> Whilst s302m multiple substreams I haven't seen, Dolby E streams internally
> contain multiple programs, often 5.1 and a 2.0 downmix.

That is downstream of the Dolby-E decoder and user will have to use a 
filter like channelsplit to bifurcate
those channels irrespective of where the s302m code resides.


> There is a 3x3 matrix of flavours between coded Dolby E and the carriage
> PCM (16-bit, 20-bit, 24-bit).

Have you seen a larger non-PCM word size inside a smaller AES3 word?

Regards,
Gyan
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Niklas Haas]

On Sat, 17 Feb 2024 18:45:57 +0530 Gyan Doshi <ffmpeg@gyani.pro> wrote:
> > Whilst s302m multiple substreams I haven't seen, Dolby E streams internally
> > contain multiple programs, often 5.1 and a 2.0 downmix.
> 
> That is downstream of the Dolby-E decoder and user will have to use a 
> filter like channelsplit to bifurcate
> those channels irrespective of where the s302m code resides.

Is there metadata in Dolby E that tells you which channels belong
together (and what attributes they might have)? Is it always 5.1+2.0, or
could you have e.g. four different 2.0 programs encoded into a single
S302M/AES3 stream?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Gyan Doshi]

On 2024-02-17 07:12 pm, Niklas Haas wrote:
> On Sat, 17 Feb 2024 18:45:57 +0530 Gyan Doshi <ffmpeg@gyani.pro> wrote:
>>> Whilst s302m multiple substreams I haven't seen, Dolby E streams internally
>>> contain multiple programs, often 5.1 and a 2.0 downmix.
>> That is downstream of the Dolby-E decoder and user will have to use a
>> filter like channelsplit to bifurcate
>> those channels irrespective of where the s302m code resides.
> Is there metadata in Dolby E that tells you which channels belong
> together (and what attributes they might have)? Is it always 5.1+2.0, or
> could you have e.g. four different 2.0 programs encoded into a single
> S302M/AES3 stream?

Yes, the metadata is program configuration, with 8 max channels and 8 
max programs, so 8 mono programs at most.
But my samples are all 5.1+2 or 7.1 or 4.0. These will all result in a 
single decoded stream from the dolby_e decoder.

Regards,
Gyan

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Kieran Kunhya]

On Sat, 17 Feb 2024 at 13:16, Gyan Doshi <ffmpeg@gyani.pro> wrote:

> > Whilst s302m multiple substreams I haven't seen, Dolby E streams
> internally
> > contain multiple programs, often 5.1 and a 2.0 downmix.
>
> That is downstream of the Dolby-E decoder and user will have to use a
> filter like channelsplit to bifurcate
> those channels irrespective of where the s302m code resides.
>

The point is there are several layers of streams that also can change
dynamically.


> > There is a 3x3 matrix of flavours between coded Dolby E and the carriage
> > PCM (16-bit, 20-bit, 24-bit).
>
> Have you seen a larger non-PCM word size inside a smaller AES3 word?
>

Actually I think that case won't work in the real world, true as the LSBs
will be truncated, not passed along.

Kieran
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1015 bytes --]

On Sat, Feb 17, 2024 at 05:15:43PM +0530, 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.

[Nested decoder, bsf, demuxer, something else ...]

I think the discussion is too much looking at the fine details

What we need is
TS -> raw samples
TS -> TS    (lossless)
TS -> nonTS (lossless)
nonTS -> TS (lossless)
raw samples -> TS

I think some of the discussed options have problems with lossless non PCM
in some intermediate encapsulation in mpeg-TS

thx

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

I do not agree with what you have to say, but I'll defend to the death your
right to say it. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Gyan Doshi]

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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] FFmpeg TC input needed

[Gyan Doshi]

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@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [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@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@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [TC] Technical decision on S302M non-PCM decoding

[Niklas Haas]

On Sat, 17 Feb 2024 17:15:43 +0530 Gyan Doshi <ffmpeg@gyani.pro> 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
> 

Hi,

This is in regards to the patch "avcodec/s302m: enable non-PCM decoding"
by Gyan Doshi.
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240127103854.9971-1-ffmpeg@gyani.pro/

The TC has voted on the matter, with all 5 voting members unanimously
voting against the patch.

The opposition to the patch was based on the opinion that this
implementation of S302M should be handled by libavformat, not
libavcodec. The central argument focused on the ways in which these
formats can interact. In particular, S302M is always tied to MPEG-TS,
but Dolby E can also exist in other containers, so exposing Dolby E as
the the intermediary (AV_CODEC_ID) unlocks the full range of possible
operations (e.g. S302M -> Dolby E -> MXF remux).

Regarding Gyan Doshi's main objection that doing so would possibly
delete S302M metadata (e.g. S-ADM), it was observed that if the need for
such metadata truly arises, it should either be parsed by FFmpeg or
attached as an opaque side data blob (only if the former is impossible).

Therefore, the matter has been decided, rejecting the patch at hand.

Regards,
The FFmpeg Technical Committee
- Jan Ekström
- Niklas Haas
- Anton Khirnov
- Michael Niedermayer
- Martin Storsjö
- Mark Thompson
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".