[FFmpeg-devel] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4

[FFmpeg-devel] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4

[Raphaël Zumer]

Encoding PCM in MP4 currently causes subsequent decoding to fail due to a sample size of 0.

Use bits per coded sample instead, which are set correctly based on my tests and allow muxed files to be decoded as expected.

Note: PCM in MP4 muxed with versions of FFmpeg 6.0 and prior (before implementation of the pcmC box) will continue to fail decoding due to the sample size not being available. I see that it was assumed to be 16-bit before commit d4ee17.

Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
---
  libavformat/movenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index f1cc80b1b3..3c44ace5b0 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -1237,7 +1237,7 @@ static int mov_write_pcmc_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
                      track->par->codec_id == AV_CODEC_ID_PCM_S24LE ||
                      track->par->codec_id == AV_CODEC_ID_PCM_S32LE);
      avio_w8(pb, format_flags);
-    avio_w8(pb, track->par->bits_per_raw_sample);
+    avio_w8(pb, track->par->bits_per_coded_sample);
  
      return update_size(pb, pos);
  }
-- 
2.41.0

_______________________________________________
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] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4

["zhilizhao(赵志立)"]

> On Jul 26, 2023, at 00:28, Raphaël Zumer <raphael.zumer@vimeo.com> wrote:
> 
> Encoding PCM in MP4 currently causes subsequent decoding to fail due to a sample size of 0.

This doesn’t give a context on which case the sample size is 0.

> Use bits per coded sample instead, which are set correctly based on my tests and allow muxed files to be decoded as expected.

Since neither bits_per_raw_sample or bits_per_coded_sample has strong guarantee,
I decided to fall back to av_get_exact_bits_per_sample().

http://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/312653.html

> 
> Note: PCM in MP4 muxed with versions of FFmpeg 6.0 and prior (before implementation of the pcmC box) will continue to fail decoding due to the sample size not being available. I see that it was assumed to be 16-bit before commit d4ee17.

The assumption is incorrect. We didn’t recognize those samples are
ISO/IEC 23003-5 before.

> 
> Signed-off-by: Raphaël Zumer <rzumer@tebako.net>
> ---
> libavformat/movenc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index f1cc80b1b3..3c44ace5b0 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -1237,7 +1237,7 @@ static int mov_write_pcmc_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra
>                     track->par->codec_id == AV_CODEC_ID_PCM_S24LE ||
>                     track->par->codec_id == AV_CODEC_ID_PCM_S32LE);
>     avio_w8(pb, format_flags);
> -    avio_w8(pb, track->par->bits_per_raw_sample);
> +    avio_w8(pb, track->par->bits_per_coded_sample);
>      return update_size(pb, pos);
> }
> -- 
> 2.41.0
> 
> _______________________________________________
> 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] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4

[Raphaël Zumer]

Hello,

On 7/26/23 00:09, "zhilizhao(赵志立)" wrote:
>> On Jul 26, 2023, at 00:28, Raphaël Zumer <raphael.zumer@vimeo.com> wrote:
>>
>> Encoding PCM in MP4 currently causes subsequent decoding to fail due to a sample size of 0.
> This doesn’t give a context on which case the sample size is 0.

Using FFmpeg (round-trip). Just mux some Wave file to MP4 and try decoding it back. Unless it is sample-dependent for some reason, it will always fail.

>> Use bits per coded sample instead, which are set correctly based on my tests and allow muxed files to be decoded as expected.
> Since neither bits_per_raw_sample or bits_per_coded_sample has strong guarantee,
> I decided to fall back to av_get_exact_bits_per_sample().
>
> http://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/312653.html

Can you show or describe a case where bits_per_raw_sample is set and not bits_per_coded_sample?

For PCM av_get_exact_bits_per_sample() looks fine, but if the behavior of bits_per_raw_sample/bits_per_coded_sample is inconsistent on decode that seems undesirable or undocumented behavior that should be rectified.

Looking at the documentation (https://ffmpeg.org/doxygen/6.0/structAVCodecParameters.html) it is mentioned to be equivalent to bits_per_raw_sample for PCM formats. So this looks like a bug.

>> Note: PCM in MP4 muxed with versions of FFmpeg 6.0 and prior (before implementation of the pcmC box) will continue to fail decoding due to the sample size not being available. I see that it was assumed to be 16-bit before commit d4ee17.
> The assumption is incorrect. We didn’t recognize those samples are
> ISO/IEC 23003-5 before.

I mean that it is possible to mux s16 PCM into MP4 (in a nonstandard way I guess) prior to that patch set and this breaks decoding files created this way.

I can mux a s16le Wave file using FFmpeg 6.0 to MP4, it will be decoded correctly and retain the sample format. Try to decode it using FFmpeg post the 23003-5 patch and it simply fails decoding because there's no pcmC box.

There doesn't need to be a fallback, just a consideration.

Thanks

Raphaël Zumer

_______________________________________________
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] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4

[Zhao Zhili]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Raphaël Zumer
> Sent: 2023年7月26日 21:20
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4
> 
> Hello,
> 
> On 7/26/23 00:09, "zhilizhao(赵志立)" wrote:
> >> On Jul 26, 2023, at 00:28, Raphaël Zumer <raphael.zumer@vimeo.com> wrote:
> >>
> >> Encoding PCM in MP4 currently causes subsequent decoding to fail due to a sample size of 0.
> > This doesn’t give a context on which case the sample size is 0.
> 
> Using FFmpeg (round-trip). Just mux some Wave file to MP4 and try decoding it back. Unless it is sample-dependent for some reason, it
> will always fail.
> 
> >> Use bits per coded sample instead, which are set correctly based on my tests and allow muxed files to be decoded as expected.
> > Since neither bits_per_raw_sample or bits_per_coded_sample has strong guarantee,
> > I decided to fall back to av_get_exact_bits_per_sample().
> >
> > http://ffmpeg.org/pipermail/ffmpeg-devel/2023-July/312653.html
> 
> Can you show or describe a case where bits_per_raw_sample is set and not bits_per_coded_sample?
> 
> For PCM av_get_exact_bits_per_sample() looks fine, but if the behavior of bits_per_raw_sample/bits_per_coded_sample is inconsistent
> on decode that seems undesirable or undocumented behavior that should be rectified.
> 
> Looking at the documentation (https://ffmpeg.org/doxygen/6.0/structAVCodecParameters.html) it is mentioned to be equivalent to
> bits_per_raw_sample for PCM formats. So this looks like a bug.
> 
> >> Note: PCM in MP4 muxed with versions of FFmpeg 6.0 and prior (before implementation of the pcmC box) will continue to fail
> decoding due to the sample size not being available. I see that it was assumed to be 16-bit before commit d4ee17.
> > The assumption is incorrect. We didn’t recognize those samples are
> > ISO/IEC 23003-5 before.
> 
> I mean that it is possible to mux s16 PCM into MP4 (in a nonstandard way I guess) prior to that patch set and this breaks decoding files
> created this way.
> 
> I can mux a s16le Wave file using FFmpeg 6.0 to MP4, it will be decoded correctly and retain the sample format. Try to decode it using
> FFmpeg post the 23003-5 patch and it simply fails decoding because there's no pcmC box.

I think you mixed up PCM in mp4 (-f mp4) and PCM in quicktime format (-f mov).

There is no support of mux PCM in mp4 before the patch. And demux PCM in mp4 only works by chance.
Let's focus on improve current situation, there isn't much to look back.

> 
> There doesn't need to be a fallback, just a consideration.
> 
> Thanks
> 
> Raphaël Zumer
> 
> _______________________________________________
> 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] [PATCH] avformat/movenc: Fix writing a sample size of 0 for PCM in MP4

[Raphaël Zumer]

On 7/26/23 11:37, Zhao Zhili wrote:
> I think you mixed up PCM in mp4 (-f mp4) and PCM in quicktime format (-f mov).
>
> There is no support of mux PCM in mp4 before the patch. And demux PCM in mp4 only works by chance.
> Let's focus on improve current situation, there isn't much to look back.

I double-checked my test sample that I thought was muxed with FFmpeg 6.0, and actually the lavf version matches the most recent one. FFmpeg 6.0 does reject it for muxing in MP4. Sorry for the mixup.

RZ


_______________________________________________
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".