[FFmpeg-devel] [PATCH] libavformat/rtpdec_mpeg: handle bare ADTS packets with explicit decoder config

2021-10-01 Thread Jeff Mahoney 
When SDP specifies a decoder config, there may not be any AU headers
provided by the sender.  This can result in rtp_parse_mp4_au failing
and aac_parse_packet reporting "Error parsing AU headers." and no audio
is recovered from the stream.

This commit modifies aac_parse_header to check for an explicit decoder config
set by the sdp parser (e.g. a:fmtp # config=hexvalue). If it has and there
is an ADTS header present, it skips the header and copies the RTP
payload directly as an AAC packet.

This resolves an issue observed with some inexpensive IP cameras.

Signed-off-by: Jeff Mahoney 
---
 libavformat/rtpdec_mpeg4.c | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 34c7950bcc..dd0ced790e 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -176,7 +176,7 @@ static int aac_parse_packet(AVFormatContext *ctx, 
PayloadContext *data,
 int flags)
 {
 int ret;
-
+AVCodecParameters *par = st->codecpar;
 
 if (!buf) {
 if (data->cur_au_index > data->nb_au_headers) {
@@ -204,6 +204,38 @@ static int aac_parse_packet(AVFormatContext *ctx, 
PayloadContext *data,
 return 1;
 }
 
+/* Check for an explicit decoder config (e.g. SDP a:fmtp... config=) */
+if (par->extradata && len > 7) {
+/*
+ * Start of ADTS header - syncword
+ * If present skip the header and copy the entire payload as AAC data
+ */
+if (buf[0] == 0xff && (buf[1] & 0xf0) == 0xf0) {
+/*
+ * The ADTS header is 7 or 9 bytes depending on whether
+ * the protection absent bit is set.  If it is unset, a 16-bit CRC
+ * is appended to the header.
+ */
+size_t header_size = 7 + ((buf[1] & 0x01) ? 0 : 2);
+if (len < header_size) {
+ac_log(ctx, AV_LOG_ERROR, "Error parsing ADTS header\n");
+return -1;
+}
+
+buf += header_size;
+len -= header_size;
+
+if ((ret = av_new_packet(pkt, len)) < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
+return ret;
+}
+memcpy(pkt->data, buf, len);
+pkt->stream_index = st->index;
+
+return 0;
+}
+}
+
 if (rtp_parse_mp4_au(data, buf, len)) {
 av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
 return -1;
-- 
2.33.0

___
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".

Re: [FFmpeg-devel] [PATCH] libavformat/rtpdec_mpeg: handle bare ADTS packets with explicit decoder config

2021-10-17 Thread Jeff Mahoney 
Hi folks -

Just wanted to follow up on this patch. Is it obviously wrong, it
doesn't scratch a common itch, or... ?  I've tested the camera
connection and ran the patched code through FATE.   The frigate
container I'm using has audio disabled entirely since it's unreliable on
many devices and this fixes it for at least some.  I'd prefer to get the
patch (in this iteration or after review/resubmit cycles) landed so the
container stack can pull it in.

I've been in the Linux world long enough to know I can't expect free
review, but if I've missed something in the process I'd appreciate some
feedback.

Thanks,

-Jeff

On 10/2/21 6:53 PM, Jeff Mahoney wrote:
> When SDP specifies a decoder config, there may not be any AU headers
> provided by the sender.  This can result in rtp_parse_mp4_au failing
> and aac_parse_packet reporting "Error parsing AU headers." and no audio
> is recovered from the stream.
> 
> This commit modifies aac_parse_header to check for an explicit decoder config
> set by the sdp parser (e.g. a:fmtp # config=hexvalue). If it has and there
> is an ADTS header present, it skips the header and copies the RTP
> payload directly as an AAC packet.
> 
> This resolves an issue observed with some inexpensive IP cameras.
> 
> Signed-off-by: Jeff Mahoney 
> ---
>  libavformat/rtpdec_mpeg4.c | 34 +-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
> index 34c7950bcc..dd0ced790e 100644
> --- a/libavformat/rtpdec_mpeg4.c
> +++ b/libavformat/rtpdec_mpeg4.c
> @@ -176,7 +176,7 @@ static int aac_parse_packet(AVFormatContext *ctx, 
> PayloadContext *data,
>  int flags)
>  {
>  int ret;
> -
> +AVCodecParameters *par = st->codecpar;
>  
>  if (!buf) {
>  if (data->cur_au_index > data->nb_au_headers) {
> @@ -204,6 +204,38 @@ static int aac_parse_packet(AVFormatContext *ctx, 
> PayloadContext *data,
>  return 1;
>  }
>  
> +/* Check for an explicit decoder config (e.g. SDP a:fmtp... config=) */
> +if (par->extradata && len > 7) {
> +/*
> + * Start of ADTS header - syncword
> + * If present skip the header and copy the entire payload as AAC data
> + */
> +if (buf[0] == 0xff && (buf[1] & 0xf0) == 0xf0) {
> +/*
> + * The ADTS header is 7 or 9 bytes depending on whether
> + * the protection absent bit is set.  If it is unset, a 16-bit 
> CRC
> + * is appended to the header.
> + */
> +size_t header_size = 7 + ((buf[1] & 0x01) ? 0 : 2);
> +if (len < header_size) {
> +ac_log(ctx, AV_LOG_ERROR, "Error parsing ADTS header\n");
> +return -1;
> +}
> +
> +buf += header_size;
> +len -= header_size;
> +
> +if ((ret = av_new_packet(pkt, len)) < 0) {
> +av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
> +return ret;
> +        }
> +memcpy(pkt->data, buf, len);
> +pkt->stream_index = st->index;
> +
> +return 0;
> +}
> +}
> +
>  if (rtp_parse_mp4_au(data, buf, len)) {
>  av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
>  return -1;
> 

-- 
--
Jeff Mahoney



OpenPGP_signature
Description: OpenPGP digital signature
___
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".

Re: [FFmpeg-devel] [PATCH] libavformat/rtpdec_mpeg: handle bare ADTS packets with explicit decoder config

2021-10-19 Thread Jeff Mahoney 
On 10/17/21 21:28, lance.lmw...@gmail.com wrote:
> On Fri, Oct 01, 2021 at 06:53:23PM -0400, Jeff Mahoney wrote:
>> When SDP specifies a decoder config, there may not be any AU headers
>> provided by the sender.  This can result in rtp_parse_mp4_au failing
>> and aac_parse_packet reporting "Error parsing AU headers." and no audio
>> is recovered from the stream.
>>
>> This commit modifies aac_parse_header to check for an explicit decoder config
>> set by the sdp parser (e.g. a:fmtp # config=hexvalue). If it has and there
>> is an ADTS header present, it skips the header and copies the RTP
>> payload directly as an AAC packet.
>>
>> This resolves an issue observed with some inexpensive IP cameras.
>>
>> Signed-off-by: Jeff Mahoney 
>> ---
>>  libavformat/rtpdec_mpeg4.c | 34 +-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
>> index 34c7950bcc..dd0ced790e 100644
>> --- a/libavformat/rtpdec_mpeg4.c
>> +++ b/libavformat/rtpdec_mpeg4.c
>> @@ -176,7 +176,7 @@ static int aac_parse_packet(AVFormatContext *ctx, 
>> PayloadContext *data,
>>  int flags)
>>  {
>>  int ret;
>> -
>> +AVCodecParameters *par = st->codecpar;
>>  
>>  if (!buf) {
>>  if (data->cur_au_index > data->nb_au_headers) {
>> @@ -204,6 +204,38 @@ static int aac_parse_packet(AVFormatContext *ctx, 
>> PayloadContext *data,
>>  return 1;
>>  }
>>  
>> +/* Check for an explicit decoder config (e.g. SDP a:fmtp... config=) */
>> +if (par->extradata && len > 7) {
>> +/*
>> + * Start of ADTS header - syncword
>> + * If present skip the header and copy the entire payload as AAC 
>> data
>> + */
>> +if (buf[0] == 0xff && (buf[1] & 0xf0) == 0xf0) {
>> +/*
>> + * The ADTS header is 7 or 9 bytes depending on whether
>> + * the protection absent bit is set.  If it is unset, a 16-bit 
>> CRC
>> + * is appended to the header.
>> + */
>> +size_t header_size = 7 + ((buf[1] & 0x01) ? 0 : 2);
>> +if (len < header_size) {
>> +ac_log(ctx, AV_LOG_ERROR, "Error parsing ADTS header\n");
>> +return -1;
>> +}
> 
> I prefer to use avpriv_adts_header_parse() for ADTS header parse.


Thanks, Lance.  This wasn't in the master branch when I posted this.
It's much cleaner and I'm happy to adapt to it.

-Jeff

>> +
>> +buf += header_size;
>> +len -= header_size;
>> +
>> +if ((ret = av_new_packet(pkt, len)) < 0) {
>> +av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
>> +return ret;
>> +}
>> +memcpy(pkt->data, buf, len);
>> +pkt->stream_index = st->index;
>> +
>> +return 0;
>> +}
>> +}
>> +
>>  if (rtp_parse_mp4_au(data, buf, len)) {
>>  av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
>>  return -1;
>> -- 
>> 2.33.0
>>
>> ___
>> 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".
> 


-- 
Jeff Mahoney
___
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] [PATCH v2] libavformat/rtpdec_mpeg: handle bare ADTS packets with explicit decoder config

2021-10-19 Thread Jeff Mahoney 
When SDP specifies a decoder config, there may not be any AU headers
provided by the sender.  This can result in rtp_parse_mp4_au failing
and aac_parse_packet reporting "Error parsing AU headers." and no audio
is recovered from the stream.

This commit modifies aac_parse_header to check for an explicit decoder config
set by the sdp parser (e.g. a:fmtp # config=hexvalue). If it has and there
is an ADTS header present, it skips the header and copies the RTP
payload directly as an AAC packet.

This resolves an issue observed with some inexpensive IP cameras.

Signed-off-by: Jeff Mahoney 
---
 libavformat/rtpdec_mpeg4.c | 37 -
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/libavformat/rtpdec_mpeg4.c b/libavformat/rtpdec_mpeg4.c
index 34c7950bcc..13d0770dd6 100644
--- a/libavformat/rtpdec_mpeg4.c
+++ b/libavformat/rtpdec_mpeg4.c
@@ -32,6 +32,8 @@
 #include "libavutil/attributes.h"
 #include "libavutil/avstring.h"
 #include "libavcodec/get_bits.h"
+#include "libavcodec/adts_header.h"
+#include "libavcodec/adts_parser.h"
 
 #define MAX_AAC_HBR_FRAME_SIZE 8191
 
@@ -176,7 +178,7 @@ static int aac_parse_packet(AVFormatContext *ctx, 
PayloadContext *data,
 int flags)
 {
 int ret;
-
+AVCodecParameters *par = st->codecpar;
 
 if (!buf) {
 if (data->cur_au_index > data->nb_au_headers) {
@@ -204,6 +206,39 @@ static int aac_parse_packet(AVFormatContext *ctx, 
PayloadContext *data,
 return 1;
 }
 
+/* Check for an explicit decoder config (e.g. SDP a:fmtp... config=) */
+if (par->extradata && len > 7) {
+AACADTSHeaderInfo *header;
+
+/*
+ * Check for ADTS header
+ * If present skip the header and copy the entire payload as AAC data
+ */
+ret = avpriv_adts_header_parse(&header, buf, len);
+if (!ret) {
+buf += AV_AAC_ADTS_HEADER_SIZE;
+len -= AV_AAC_ADTS_HEADER_SIZE;
+
+/* Skip 16-bit CRC if present */
+if (!header->crc_absent) {
+buf += 2;
+len -= 2;
+}
+av_free(header);
+
+ret = av_new_packet(pkt, len);
+if (ret < 0) {
+av_log(ctx, AV_LOG_ERROR, "Out of memory\n");
+return ret;
+}
+
+memcpy(pkt->data, buf, len);
+pkt->stream_index = st->index;
+
+return 0;
+}
+}
+
 if (rtp_parse_mp4_au(data, buf, len)) {
 av_log(ctx, AV_LOG_ERROR, "Error parsing AU headers\n");
 return -1;
-- 
2.33.1

___
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".