On 28 June 2010 01:51, Martin Storsjö <[email protected]> wrote:
> On Mon, 28 Jun 2010, Josh Allmann wrote:
>>
>> Xiph and AMR cleanups attached, but I'd appreciate it if someone could
>> test the AMR and Vorbis audio stuff for me.
>
> The AMR cleanup isn't totally right. Btw - the sample_50kbit.3gp sample in
> DSS has AMR, you can test with that.
>
Seems OK.
> I have no Vorbis testing setup at the moment, but isn't that quite easy to
> set up with feng?
>
Before/after CRCs are the same, so I suppose it works.
> These variables, octet_align, crc, interleaving, channels, were local to
> the amr_parse_sdp_line function earlier - now they're local to
> amr_parse_fmtp. This means that they're reset to the default values after
> each invocation of amr_parse_fmtp, and we're unable to verify that the
> complete fmtp line had a valid configuration.
>
> In practice, this would trigger a warning after parsing any other
> configuration item than octet-align, since octet_align would be left at 0
> when parsing those other configuration items. The sample only has
> octet-align and no other configuration items, though, so you wouldn't hit
> this issue with that.
>
> So those variables should be moved to the payload context in this case -
> even if they're not used by the packet parsing routine (yet at least).
>
10l to me. Fixed.
>> + if (av_strstart(line, "fmtp:", &p)) {
>> + return ff_parse_fmtp(s->streams[st_index], data, p,
>> + &amr_parse_fmtp);
>> }
>
> Btw, the common coding style is to omit & for function pointers, I think.
Corrected in the Xiph and AMR patches.
Josh
From 3f7008e7b8566140183fd19281b3d09cc3965f59 Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Mon, 28 Jun 2010 11:39:03 -0700
Subject: [PATCH 1/4] malloc fmtp value
---
libavformat/rtpdec.c | 15 ++++++++++++---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 74858f6..4881832 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -538,8 +538,14 @@ int ff_parse_fmtp(AVStream *stream, PayloadContext *data, const char *p,
char *attr, char *value))
{
char attr[256];
- char value[4096];
+ char *value;
int res;
+ int value_size = strlen(p);
+
+ if (!(value = av_malloc(value_size))) {
+ av_log(stream, AV_LOG_ERROR, "Failed to allocate data for FMTP.");
+ return AVERROR(ENOMEM);
+ }
// remove protocol identifier
while (*p && *p == ' ') p++; // strip spaces
@@ -548,11 +554,14 @@ int ff_parse_fmtp(AVStream *stream, PayloadContext *data, const char *p,
while (ff_rtsp_next_attr_and_value(&p,
attr, sizeof(attr),
- value, sizeof(value))) {
+ value, value_size)) {
res = parse_fmtp(stream, data, attr, value);
- if (res < 0)
+ if (res < 0 && res != AVERROR_PATCHWELCOME) {
+ av_free(value);
return res;
+ }
}
+ av_free(value);
return 0;
}
--
1.7.0.4
From d7cbb601c38539aae307bbc321027aa520a69b3e Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Mon, 28 Jun 2010 11:39:41 -0700
Subject: [PATCH 2/4] Cleanup FMTP parsing code in Xiph RTP depacketizer.
---
libavformat/rtpdec_xiph.c | 29 ++++-------------------------
1 files changed, 4 insertions(+), 25 deletions(-)
diff --git a/libavformat/rtpdec_xiph.c b/libavformat/rtpdec_xiph.c
index 34704a2..9973efc 100644
--- a/libavformat/rtpdec_xiph.c
+++ b/libavformat/rtpdec_xiph.c
@@ -286,10 +286,11 @@ parse_packed_headers(const uint8_t * packed_headers,
return 0;
}
-static int xiph_parse_fmtp_pair(AVCodecContext * codec,
+static int xiph_parse_fmtp_pair(AVStream* stream,
PayloadContext *xiph_data,
char *attr, char *value)
{
+ AVCodecContext *codec = stream->codec;
int result = 0;
if (!strcmp(attr, "sampling")) {
@@ -346,34 +347,12 @@ static int xiph_parse_sdp_line(AVFormatContext *s, int st_index,
PayloadContext *data, const char *line)
{
const char *p;
- char *value;
- char attr[25];
- int value_size = strlen(line), attr_size = sizeof(attr), res = 0;
- AVCodecContext* codec = s->streams[st_index]->codec;
-
- assert(data);
-
- if (!(value = av_malloc(value_size))) {
- av_log(codec, AV_LOG_ERROR, "Out of memory\n");
- return AVERROR(ENOMEM);
- }
if (av_strstart(line, "fmtp:", &p)) {
- // remove protocol identifier
- while (*p && *p == ' ') p++; // strip spaces
- while (*p && *p != ' ') p++; // eat protocol identifier
- while (*p && *p == ' ') p++; // strip trailing spaces
-
- while (ff_rtsp_next_attr_and_value(&p,
- attr, attr_size,
- value, value_size)) {
- res = xiph_parse_fmtp_pair(codec, data, attr, value);
- if (res < 0 && res != AVERROR_PATCHWELCOME)
- return res;
- }
+ return ff_parse_fmtp(s->streams[st_index], data, p,
+ xiph_parse_fmtp_pair);
}
- av_free(value);
return 0;
}
--
1.7.0.4
From c8b7b88ad4c727880d246566ab813662f6a26cbb Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Mon, 28 Jun 2010 11:40:30 -0700
Subject: [PATCH 3/4] Clean up FMTP parsing code in AMR RTP depacketizer.
---
libavformat/rtpdec_amr.c | 79 +++++++++++++++++++++++++++++-----------------
1 files changed, 50 insertions(+), 29 deletions(-)
diff --git a/libavformat/rtpdec_amr.c b/libavformat/rtpdec_amr.c
index a7b36c7..d3dbf60 100644
--- a/libavformat/rtpdec_amr.c
+++ b/libavformat/rtpdec_amr.c
@@ -30,6 +30,26 @@ static const uint8_t frame_sizes_wb[16] = {
17, 23, 32, 36, 40, 46, 50, 58, 60, 5, 5, 0, 0, 0, 0, 0
};
+struct PayloadContext {
+ int octet_align;
+ int crc;
+ int interleaving;
+ int channels;
+};
+
+static PayloadContext *amr_new_context(void)
+{
+ PayloadContext *data = av_mallocz(sizeof(PayloadContext));
+ if(!data) return data;
+ data->channels = 1;
+ return data;
+}
+
+static void amr_free_context(PayloadContext *data)
+{
+ av_free(data);
+}
+
static int amr_handle_packet(AVFormatContext *ctx,
PayloadContext *data,
AVStream *st,
@@ -120,50 +140,47 @@ static int amr_handle_packet(AVFormatContext *ctx,
return 0;
}
-static int amr_parse_sdp_line(AVFormatContext *s, int st_index,
- PayloadContext *data, const char *line)
+static int amr_parse_fmtp(AVStream *stream, PayloadContext *data,
+ char *attr, char *value)
{
- const char *p;
- char attr[25], value[25];
-
- /* Parse an fmtp line this one:
- * a=fmtp:97 octet-align=1; interleaving=0
- * That is, a normal fmtp: line followed by semicolon & space
- * separated key/value pairs.
- */
- if (av_strstart(line, "fmtp:", &p)) {
- int octet_align = 0;
- int crc = 0;
- int interleaving = 0;
- int channels = 1;
-
- while (*p && *p == ' ') p++; /* strip spaces */
- while (*p && *p != ' ') p++; /* eat protocol identifier */
- while (*p && *p == ' ') p++; /* strip trailing spaces */
-
- while (ff_rtsp_next_attr_and_value(&p, attr, sizeof(attr), value, sizeof(value))) {
/* Some AMR SDP configurations contain "octet-align", without
* the trailing =1. Therefore, if the value is empty,
* interpret it as "1".
*/
if (!strcmp(value, "")) {
- av_log(s, AV_LOG_WARNING, "AMR fmtp attribute %s had "
+ av_log(stream, AV_LOG_WARNING, "AMR fmtp attribute %s had "
"nonstandard empty value\n", attr);
strcpy(value, "1");
}
if (!strcmp(attr, "octet-align"))
- octet_align = atoi(value);
+ data->octet_align = atoi(value);
else if (!strcmp(attr, "crc"))
- crc = atoi(value);
+ data->crc = atoi(value);
else if (!strcmp(attr, "interleaving"))
- interleaving = atoi(value);
+ data->interleaving = atoi(value);
else if (!strcmp(attr, "channels"))
- channels = atoi(value);
- }
- if (!octet_align || crc || interleaving || channels != 1) {
- av_log(s, AV_LOG_ERROR, "Unsupported RTP/AMR configuration!\n");
+ data->channels = atoi(value);
+ if (!data->octet_align || data->crc ||
+ data->interleaving || data->channels != 1) {
+ av_log(stream, AV_LOG_ERROR, "Unsupported RTP/AMR configuration!\n");
return -1;
}
+ return 0;
+}
+
+static int amr_parse_sdp_line(AVFormatContext *s, int st_index,
+ PayloadContext *data, const char *line)
+{
+ const char *p;
+
+ /* Parse an fmtp line this one:
+ * a=fmtp:97 octet-align=1; interleaving=0
+ * That is, a normal fmtp: line followed by semicolon & space
+ * separated key/value pairs.
+ */
+ if (av_strstart(line, "fmtp:", &p)) {
+ return ff_parse_fmtp(s->streams[st_index], data, p,
+ amr_parse_fmtp);
}
return 0;
}
@@ -173,6 +190,8 @@ RTPDynamicProtocolHandler ff_amr_nb_dynamic_handler = {
.codec_type = AVMEDIA_TYPE_AUDIO,
.codec_id = CODEC_ID_AMR_NB,
.parse_sdp_a_line = amr_parse_sdp_line,
+ .open = amr_new_context,
+ .close = amr_free_context,
.parse_packet = amr_handle_packet,
};
@@ -181,6 +200,8 @@ RTPDynamicProtocolHandler ff_amr_wb_dynamic_handler = {
.codec_type = AVMEDIA_TYPE_AUDIO,
.codec_id = CODEC_ID_AMR_WB,
.parse_sdp_a_line = amr_parse_sdp_line,
+ .open = amr_new_context,
+ .close = amr_free_context,
.parse_packet = amr_handle_packet,
};
--
1.7.0.4
From 8aafebdf85519ae4da1a9b2a296408ec87c24b2e Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Mon, 28 Jun 2010 11:42:57 -0700
Subject: [PATCH 4/4] amr rtp cosmetics
---
libavformat/rtpdec_amr.c | 46 ++++++++++++++++++++++++----------------------
1 files changed, 24 insertions(+), 22 deletions(-)
diff --git a/libavformat/rtpdec_amr.c b/libavformat/rtpdec_amr.c
index d3dbf60..e2bd9d6 100644
--- a/libavformat/rtpdec_amr.c
+++ b/libavformat/rtpdec_amr.c
@@ -143,28 +143,30 @@ static int amr_handle_packet(AVFormatContext *ctx,
static int amr_parse_fmtp(AVStream *stream, PayloadContext *data,
char *attr, char *value)
{
- /* Some AMR SDP configurations contain "octet-align", without
- * the trailing =1. Therefore, if the value is empty,
- * interpret it as "1".
- */
- if (!strcmp(value, "")) {
- av_log(stream, AV_LOG_WARNING, "AMR fmtp attribute %s had "
- "nonstandard empty value\n", attr);
- strcpy(value, "1");
- }
- if (!strcmp(attr, "octet-align"))
- data->octet_align = atoi(value);
- else if (!strcmp(attr, "crc"))
- data->crc = atoi(value);
- else if (!strcmp(attr, "interleaving"))
- data->interleaving = atoi(value);
- else if (!strcmp(attr, "channels"))
- data->channels = atoi(value);
- if (!data->octet_align || data->crc ||
- data->interleaving || data->channels != 1) {
- av_log(stream, AV_LOG_ERROR, "Unsupported RTP/AMR configuration!\n");
- return -1;
- }
+ /* Some AMR SDP configurations contain "octet-align", without
+ * the trailing =1. Therefore, if the value is empty,
+ * interpret it as "1".
+ */
+ if (!strcmp(value, "")) {
+ av_log(stream, AV_LOG_WARNING, "AMR fmtp attribute %s had "
+ "nonstandard empty value\n", attr);
+ strcpy(value, "1");
+ }
+
+ if (!strcmp(attr, "octet-align"))
+ data->octet_align = atoi(value);
+ else if (!strcmp(attr, "crc"))
+ data->crc = atoi(value);
+ else if (!strcmp(attr, "interleaving"))
+ data->interleaving = atoi(value);
+ else if (!strcmp(attr, "channels"))
+ data->channels = atoi(value);
+ if (!data->octet_align || data->crc ||
+ data->interleaving || data->channels != 1) {
+ av_log(stream, AV_LOG_ERROR, "Unsupported RTP/AMR configuration!\n");
+ return -1;
+ }
+
return 0;
}
--
1.7.0.4
_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
