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, > + &side_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