[FFmpeg-devel] Notify user when encode a CBR mp3 file without "-write_xing false". This may result in mp3 file duration incorect on ios safari browser and webview. I try to fix it but it can’t be done as mp3 muxer don’t know it is a CBR file until encode

Submitted by sharpbai@gmail.com on April 21, 2017, 2:59 p.m.

Details

Message ID 1492786781-77792-1-git-send-email-sharpbai@gmail.com
State New
Headers show

Commit Message

sharpbai@gmail.com April 21, 2017, 2:59 p.m.
From: sharpbai <sharpbai@gmail.com>

Bug example:

ffmpeg -i a.mp3 -c:a mp3 -ab 32k -ar 44100 -ac 1 b.mp3

The duration of the generated file b.mp3 is wrong on ios safari browser from ios7 to ios10.
---
 libavformat/mp3enc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Michael Niedermayer April 22, 2017, 4:29 p.m.
On Fri, Apr 21, 2017 at 10:59:41PM +0800, sharpbai@gmail.com wrote:
> From: sharpbai <sharpbai@gmail.com>
> 
> Bug example:
> 
> ffmpeg -i a.mp3 -c:a mp3 -ab 32k -ar 44100 -ac 1 b.mp3
> 
> The duration of the generated file b.mp3 is wrong on ios safari browser from ios7 to ios10.
> ---
>  libavformat/mp3enc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

ios safari browser doesnt support the xing header ?

[...]
sharpbai@gmail.com April 23, 2017, 4:26 a.m.
ios safari browser support the xing header, and the duration of the
mp3 file with xing header is correct. But with "Info" replaced "Xing"
in xing header in CBR mode, ios safari browser does not skip the first
frame and using the first frame header(the bitrate in header has
changed to enlarge frame size as xing frame needs at least 177 bytes)
to calculate duration.

2017-04-23 0:29 GMT+08:00 Michael Niedermayer <michael@niedermayer.cc>:
> On Fri, Apr 21, 2017 at 10:59:41PM +0800, sharpbai@gmail.com wrote:
>> From: sharpbai <sharpbai@gmail.com>
>>
>> Bug example:
>>
>> ffmpeg -i a.mp3 -c:a mp3 -ab 32k -ar 44100 -ac 1 b.mp3
>>
>> The duration of the generated file b.mp3 is wrong on ios safari browser from ios7 to ios10.
>> ---
>>  libavformat/mp3enc.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> ios safari browser doesnt support the xing header ?
>
> [...]
>
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
wm4 April 23, 2017, 3:30 p.m.
On Fri, 21 Apr 2017 22:59:41 +0800
sharpbai@gmail.com wrote:

> From: sharpbai <sharpbai@gmail.com>
> 
> Bug example:
> 
> ffmpeg -i a.mp3 -c:a mp3 -ab 32k -ar 44100 -ac 1 b.mp3
> 
> The duration of the generated file b.mp3 is wrong on ios safari browser from ios7 to ios10.
> ---
>  libavformat/mp3enc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
> index 3cbf7bd..a55dbf4 100644
> --- a/libavformat/mp3enc.c
> +++ b/libavformat/mp3enc.c
> @@ -395,8 +395,13 @@ static void mp3_update_xing(AVFormatContext *s)
>      int i, rg_size;
>  
>      /* replace "Xing" identification string with "Info" for CBR files. */
> -    if (!mp3->has_variable_bitrate)
> +    if (!mp3->has_variable_bitrate) {
>          AV_WL32(mp3->xing_frame + mp3->xing_offset, MKTAG('I', 'n', 'f', 'o'));
> +        av_log(s, AV_LOG_WARNING,
> +            "This is a CBR mp3 file. But its first frame header might be wrong, "
> +            "which result in file duration incorrect on ios browser. "
> +            "Please add \"-write_xing false\" to avoid this problem.\n");
> +    }
>  
>      AV_WB32(mp3->xing_frame + mp3->xing_offset + 8,  mp3->frames);
>      AV_WB32(mp3->xing_frame + mp3->xing_offset + 12, mp3->size);

What's "ios browser". I'm not sure what this message is supposed to
help either - I think until now we usually haven't added warnings when
using or not using options that result in problems with specific
consuming applications.

The commit message subject line should not be longer than 72 characters.

Patch hide | download patch | download mbox

diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index 3cbf7bd..a55dbf4 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -395,8 +395,13 @@  static void mp3_update_xing(AVFormatContext *s)
     int i, rg_size;
 
     /* replace "Xing" identification string with "Info" for CBR files. */
-    if (!mp3->has_variable_bitrate)
+    if (!mp3->has_variable_bitrate) {
         AV_WL32(mp3->xing_frame + mp3->xing_offset, MKTAG('I', 'n', 'f', 'o'));
+        av_log(s, AV_LOG_WARNING,
+            "This is a CBR mp3 file. But its first frame header might be wrong, "
+            "which result in file duration incorrect on ios browser. "
+            "Please add \"-write_xing false\" to avoid this problem.\n");
+    }
 
     AV_WB32(mp3->xing_frame + mp3->xing_offset + 8,  mp3->frames);
     AV_WB32(mp3->xing_frame + mp3->xing_offset + 12, mp3->size);