[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 at 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