[FFmpeg-devel] [PATCH 2/2] lavf/mp3enc: write encoder delay/padding upon closing

2016-10-17 Thread Jon Toohill 
---
 libavformat/mp3enc.c | 34 --
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index de63401..4c97fa1 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -111,6 +111,8 @@ typedef struct MP3Context {
 uint64_t bag[XING_NUM_BAGS];
 int initial_bitrate;
 int has_variable_bitrate;
+int delay;
+int padding;
 
 /* index of the audio stream */
 int audio_stream_idx;
@@ -247,12 +249,7 @@ static int mp3_write_xing(AVFormatContext *s)
 ffio_fill(dyn_ctx, 0, 8); // empty replaygain fields
 avio_w8(dyn_ctx, 0);  // unknown encoding flags
 avio_w8(dyn_ctx, 0);  // unknown abr/minimal bitrate
-
-// encoder delay
-if (par->initial_padding - 528 - 1 >= 1 << 12) {
-av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n");
-}
-avio_wb24(dyn_ctx, FFMAX(par->initial_padding - 528 - 1, 0)<<12);
+avio_wb24(dyn_ctx, 0);// empty encoder delay/padding
 
 avio_w8(dyn_ctx,   0); // misc
 avio_w8(dyn_ctx,   0); // mp3gain
@@ -345,10 +342,24 @@ static int mp3_write_audio_packet(AVFormatContext *s, 
AVPacket *pkt)
 #endif
 
 if (mp3->xing_offset) {
+uint8_t *side_data = NULL;
+int side_data_size = 0;
+
 mp3_xing_add_frame(mp3, pkt);
 mp3->audio_size += pkt->size;
 mp3->audio_crc   = av_crc(av_crc_get_table(AV_CRC_16_ANSI_LE),
   mp3->audio_crc, pkt->data, pkt->size);
+
+side_data = av_packet_get_side_data(pkt,
+AV_PKT_DATA_SKIP_SAMPLES,
+_data_size);
+if (side_data && side_data_size >= 10) {
+mp3->padding = FFMAX(AV_RL32(side_data + 4) + 528 + 1, 0);
+if (!mp3->delay)
+mp3->delay =  FFMAX(AV_RL32(side_data) - 528 - 1, 0);
+} else {
+mp3->padding = 0;
+}
 }
 }
 
@@ -422,6 +433,17 @@ static void mp3_update_xing(AVFormatContext *s)
 }
 }
 
+/* write encoder delay/padding */
+if (mp3->delay >= 1 << 12) {
+mp3->delay = (1 << 12) - 1;
+av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n");
+}
+if (mp3->padding >= 1 << 12) {
+mp3->padding = (1 << 12) - 1;
+av_log(s, AV_LOG_WARNING, "Too many samples of trailing padding.\n");
+}
+AV_WB24(mp3->xing_frame + mp3->xing_offset + 141, (mp3->delay << 12) + 
mp3->padding);
+
 AV_WB32(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 8, 
mp3->audio_size);
 AV_WB16(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 4, 
mp3->audio_crc);
 
-- 
2.8.0.rc3.226.g39d4020

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH 2/2] lavf/mp3enc: write encoder delay/padding upon closing

2016-10-15 Thread Andreas Cadhalpun 
On 05.10.2016 21:37, Jon Toohill wrote:
> ---
>  libavformat/mp3enc.c | 34 +++---
>  1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
> index de63401..ddf4b93 100644
> --- a/libavformat/mp3enc.c
> +++ b/libavformat/mp3enc.c
[...]
> @@ -345,10 +342,24 @@ static int mp3_write_audio_packet(AVFormatContext *s, 
> AVPacket *pkt)
>  #endif
>  
>  if (mp3->xing_offset) {
> +uint8_t *side_data = NULL;
> +int side_data_size = 0;
> +
>  mp3_xing_add_frame(mp3, pkt);
>  mp3->audio_size += pkt->size;
>  mp3->audio_crc   = av_crc(av_crc_get_table(AV_CRC_16_ANSI_LE),
>mp3->audio_crc, pkt->data, pkt->size);
> +
> +side_data = av_packet_get_side_data(pkt,
> +AV_PKT_DATA_SKIP_SAMPLES,
> +_data_size);
> +if (side_data && side_data_size >= 10) {
> +mp3->padding = AV_RL32(side_data + 4) + 528 + 1;

I think this should also use FFMAX(..., 0), as the side_data could 
theoretically contain
a negative value.

> +if (!mp3->delay)
> +mp3->delay =  FFMAX(AV_RL32(side_data) - 528 - 1, 0);
> +} else {
> +mp3->padding = 0;
> +}
>  }
>  }
>  
> @@ -381,7 +392,7 @@ static void mp3_update_xing(AVFormatContext *s)
>  AVReplayGain *rg;
>  uint16_t tag_crc;
>  uint8_t *toc;
> -int i, rg_size;
> +int i, rg_size, delay;

This variable is not used.

>  /* replace "Xing" identification string with "Info" for CBR files. */
>  if (!mp3->has_variable_bitrate)
> @@ -422,6 +433,15 @@ static void mp3_update_xing(AVFormatContext *s)
>  }
>  }
>  
> +/* write encoder delay/padding */
> +if (mp3->delay >= 1 << 12) {
> +av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n");

I think this should cap mp3->delay to prevent setting unrelated bits in the 
header:
   mp3->delay = (1 << 12) - 1;

> +}
> +if (mp3->padding >= 1 << 12) {
> +av_log(s, AV_LOG_WARNING, "Too many samples of trailing padding.\n");

Same as above.

> +}
> +AV_WB24(mp3->xing_frame + mp3->xing_offset + 141, (mp3->delay << 12) + 
> mp3->padding);
> +
>  AV_WB32(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 8, 
> mp3->audio_size);
>  AV_WB16(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 4, 
> mp3->audio_crc);
>  
> 

Otherwise this patch looks good (and is definitely better than my previous 
attempt
of fixing this [1]).

Best regards,
Andreas


1: https://ffmpeg.org/pipermail/ffmpeg-devel/2015-December/185657.html
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

[FFmpeg-devel] [PATCH 2/2] lavf/mp3enc: write encoder delay/padding upon closing

2016-10-05 Thread Jon Toohill 
---
 libavformat/mp3enc.c | 34 +++---
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/libavformat/mp3enc.c b/libavformat/mp3enc.c
index de63401..ddf4b93 100644
--- a/libavformat/mp3enc.c
+++ b/libavformat/mp3enc.c
@@ -111,6 +111,8 @@ typedef struct MP3Context {
 uint64_t bag[XING_NUM_BAGS];
 int initial_bitrate;
 int has_variable_bitrate;
+int delay;
+int padding;
 
 /* index of the audio stream */
 int audio_stream_idx;
@@ -247,12 +249,7 @@ static int mp3_write_xing(AVFormatContext *s)
 ffio_fill(dyn_ctx, 0, 8); // empty replaygain fields
 avio_w8(dyn_ctx, 0);  // unknown encoding flags
 avio_w8(dyn_ctx, 0);  // unknown abr/minimal bitrate
-
-// encoder delay
-if (par->initial_padding - 528 - 1 >= 1 << 12) {
-av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n");
-}
-avio_wb24(dyn_ctx, FFMAX(par->initial_padding - 528 - 1, 0)<<12);
+avio_wb24(dyn_ctx, 0);// empty encoder delay/padding
 
 avio_w8(dyn_ctx,   0); // misc
 avio_w8(dyn_ctx,   0); // mp3gain
@@ -345,10 +342,24 @@ static int mp3_write_audio_packet(AVFormatContext *s, 
AVPacket *pkt)
 #endif
 
 if (mp3->xing_offset) {
+uint8_t *side_data = NULL;
+int side_data_size = 0;
+
 mp3_xing_add_frame(mp3, pkt);
 mp3->audio_size += pkt->size;
 mp3->audio_crc   = av_crc(av_crc_get_table(AV_CRC_16_ANSI_LE),
   mp3->audio_crc, pkt->data, pkt->size);
+
+side_data = av_packet_get_side_data(pkt,
+AV_PKT_DATA_SKIP_SAMPLES,
+_data_size);
+if (side_data && side_data_size >= 10) {
+mp3->padding = AV_RL32(side_data + 4) + 528 + 1;
+if (!mp3->delay)
+mp3->delay =  FFMAX(AV_RL32(side_data) - 528 - 1, 0);
+} else {
+mp3->padding = 0;
+}
 }
 }
 
@@ -381,7 +392,7 @@ static void mp3_update_xing(AVFormatContext *s)
 AVReplayGain *rg;
 uint16_t tag_crc;
 uint8_t *toc;
-int i, rg_size;
+int i, rg_size, delay;
 
 /* replace "Xing" identification string with "Info" for CBR files. */
 if (!mp3->has_variable_bitrate)
@@ -422,6 +433,15 @@ static void mp3_update_xing(AVFormatContext *s)
 }
 }
 
+/* write encoder delay/padding */
+if (mp3->delay >= 1 << 12) {
+av_log(s, AV_LOG_WARNING, "Too many samples of initial padding.\n");
+}
+if (mp3->padding >= 1 << 12) {
+av_log(s, AV_LOG_WARNING, "Too many samples of trailing padding.\n");
+}
+AV_WB24(mp3->xing_frame + mp3->xing_offset + 141, (mp3->delay << 12) + 
mp3->padding);
+
 AV_WB32(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 8, 
mp3->audio_size);
 AV_WB16(mp3->xing_frame + mp3->xing_offset + XING_SIZE - 4, 
mp3->audio_crc);
 
-- 
2.8.0.rc3.226.g39d4020

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel