Hello, Just a reminder about this patch If you need something else to validate it, please let me know
Olivier On Wed, May 22, 2019 at 6:10 PM Olivier Maignial <olivier.maign...@smile.fr> wrote: > === PROBLEM === > > I was trying to record h264 + aac streams from an RTSP server to mp4 file. > using this command line: > ffmpeg -v verbose -y -i "rtsp://<ip>/my_resources" -codec copy -bsf:a > aac_adtstoasc test.mp4 > > FFmpeg then fail to record audio and output this logs: > [rtsp @ 0xcda1f0] The profile-level-id field size is invalid (40) > [rtsp @ 0xcda1f0] Error parsing AU headers > ... > [rtsp @ 0xcda1f0] Could not find codec parameters for stream 1 (Audio: > aac, 48000 Hz, 1 channels): unspecified sample format > > In SDP provided by my RTSP server I had this fmtp line: > a=fmtp:98 streamType=5; profile-level-id=40; mode=AAC-hbr; > config=1188; sizeLength=13; indexLength=3; indexDeltaLength=3; > > In FFmpeg code, I found a check introduced by commit > 24130234cd9dd733116d17b724ea4c8e12ce097a. It disallows values greater than > 32 for fmtp line parameters. > However, In RFC-6416 (RTP Payload Format for MPEG-4 Audio/Visual Streams) > give examples of "profile-level-id" values for AAC, up to 55. > Furthermore, RFC-4566 (SDP: Session Description Protocol) do not give any > limit of size on interger parameters given in fmtp line. > > === FIX === > > Instead of prohibit values over 32, I propose to check the possible > integer overflow. > The use of strtol allow to check the string validity and the possible > overflow. > > To store and check return of strtol I use "long int" type and LONG_MIN/MAX > definitions despite differences on 32/64bits platforms. > It is consistent with the strtol man page, and it is the only way to check > if overflow or underflow is detected by strtol. > As the value is later checked against INT32_MIN and INT32_MAX, the > behavior will finnaly be the same on both type of platform. > > This patch fix my problem and I now can record my RTSP AAC stream to mp4. > It has passed the full fate tests suite sucessfully. > > Signed-off-by: Olivier Maignial <olivier.maign...@smile.fr> > --- > > Changes V4 -> V5: > - Check value against INT32_MAX/MIN instead of INT_MAX/MIN which can > be defferent depending on platform > > libavformat/rtpdec_mpeg4.c | 28 +++++++++++++++++++++++----- > 1 file changed, 23 insertions(+), 5 deletions(-) > > diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c > index 4f70599..cf35afb 100644 > --- a/libavformat/rtpdec_mpeg4.c > +++ b/libavformat/rtpdec_mpeg4.c > @@ -289,15 +289,33 @@ static int parse_fmtp(AVFormatContext *s, > for (i = 0; attr_names[i].str; ++i) { > if (!av_strcasecmp(attr, attr_names[i].str)) { > if (attr_names[i].type == ATTR_NAME_TYPE_INT) { > - int val = atoi(value); > - if (val > 32) { > + char *end_ptr = NULL; > + errno = 0; > + long int val = strtol(value, &end_ptr, 10); > + if (value[0] == '\n' || end_ptr[0] != '\0') { > av_log(s, AV_LOG_ERROR, > - "The %s field size is invalid (%d)\n", > - attr, val); > + "The %s field value is not a number > (%s)\n", > + attr, value); > return AVERROR_INVALIDDATA; > } > + if ((val == LONG_MAX && errno == ERANGE) || > + val > INT32_MAX) { > + av_log(s, AV_LOG_ERROR, > + "Value of field %s overflows maximum > integer value.\n", > + attr); > + return AVERROR_INVALIDDATA; > + } > + if ((val == LONG_MIN && errno == ERANGE) || > + val < INT32_MIN) > + { > + av_log(s, AV_LOG_ERROR, > + "Value of field %s underflows minimum > integer value.\n", > + attr); > + return AVERROR_INVALIDDATA; > + } > + > *(int *)((char *)data+ > - attr_names[i].offset) = val; > + attr_names[i].offset) = (int) val; > } else if (attr_names[i].type == ATTR_NAME_TYPE_STR) { > char *val = av_strdup(value); > if (!val) > -- > 2.7.4 > > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".