This seems reasonable to me, but I have not been involved in any rtp code.
The commit message is maybe a bit on the verbose side, but I don't have any
strong opinion.
On 12.07.2019, at 08:40, 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 strtoll allow to check the string validity and the possible
> overflow.
> Value is then checked against INT32_MIN and INT32_MAX. Using INT32_MIN/MAX
> ensure to have the same behavior on 32 or 64 bits platforms.
>
> 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 V6 --> V7:
> - Use long long int and strtoll. LLONG_MAX is always greather than
> INT32_MIN while LONG_MAX can be equal to INT32_MAX. It allows to accept full
> INT32 range.
> - Avoid to use errno
>
> libavformat/rtpdec_mpeg4.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 4f70599..f1cbedf 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -289,15 +289,18 @@ 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;
> + long long int val = strtoll(value, &end_ptr, 10);
> + if (end_ptr == value || end_ptr[0] != '\0' ||
> + val < INT32_MIN || val > INT32_MAX) {
> av_log(s, AV_LOG_ERROR,
> - "The %s field size is invalid (%d)\n",
> - attr, val);
> + "The %s field value is not a valid number, or
> overflows int32: %s\n",
> + attr, value);
> 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".
_______________________________________________
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".
