On 02/08/2015 10:22 PM, Gilles Chanteperdrix wrote:
---
libavformat/Makefile | 1 +
libavformat/rtpdec.c | 1 +
libavformat/rtpdec_ac3.c | 166 +++++++++++++++++++++++++++++++++++++++++++
libavformat/rtpdec_formats.h | 1 +
4 files changed, 169 insertions(+)
create mode 100644 libavformat/rtpdec_ac3.c
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 6bf0761..5acb292 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -30,6 +30,7 @@ OBJS-$(CONFIG_RIFFENC) += riffenc.o
OBJS-$(CONFIG_RTPDEC) += rdt.o \
rtp.o \
rtpdec.o \
+ rtpdec_ac3.o \
rtpdec_amr.o \
rtpdec_asf.o \
rtpdec_g726.o \
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 33205aa..f5557d8 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -66,6 +66,7 @@ void
ff_register_dynamic_payload_handler(RTPDynamicProtocolHandler *handler)
void ff_register_rtp_dynamic_payload_handlers(void)
{
+ ff_register_dynamic_payload_handler(&ff_ac3_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_amr_nb_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_amr_wb_dynamic_handler);
ff_register_dynamic_payload_handler(&ff_g726_16_dynamic_handler);
diff --git a/libavformat/rtpdec_ac3.c b/libavformat/rtpdec_ac3.c
new file mode 100644
index 0000000..2a47b75
--- /dev/null
+++ b/libavformat/rtpdec_ac3.c
@@ -0,0 +1,166 @@
+/*
+ * RTP parser for AC3 payload format
I would suggest to mention the RFC similar to other parsers, e.g.:
"RTP parser for AC3 payload format (RFC 4184)"
+ * Copyright (c) 2015 Gilles Chanteperdrix <g...@xenomai.org>
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "avformat.h"
+#include "rtpdec_formats.h"
+#include "libavcodec/get_bits.h"
+
Okay.
+/*
+ * From:
+ * https://tools.ietf.org/html/draft-ietf-avt-rtp-ac3-07
+ */
Could you remove this and mention the RFC earlier as suggested above?
(I would rather refer to rfc4184 from 2005 instead of the last draft
version.)
+
+struct PayloadContext {
+ unsigned nr_frames;
+ unsigned last_frame;
+ uint32_t timestamp;
+ AVIOContext *fragment;
+};
+
Okay.
+static av_cold int
+ac3_init(AVFormatContext *s, int st_index, PayloadContext *data)
I would not break the line right after the "int".
+{
+ if (st_index < 0)
+ return 0;
+ s->streams[st_index]->need_parsing = AVSTREAM_PARSE_FULL;
+ return 0;
+}
+
+static PayloadContext *ac3_new_context(void)
+{
+ return av_mallocz(sizeof(PayloadContext));
+}
+
Okay.
+static inline void free_fragment_if_needed(PayloadContext *data)
+{
+ if (data->fragment) {
+ uint8_t *p;
+ avio_close_dyn_buf(data->fragment, &p);
+ av_free(p);
+ data->fragment = NULL;
+ }
+}
This sequence is used for several RTP parsers now.
Maybe we should create a general function for these lines (separate
patch)...
+
+static void ac3_free_context(PayloadContext *data)
+{
+ free_fragment_if_needed(data);
+ av_free(data);
+}
+
+static int ac3_handle_packet(AVFormatContext *ctx, PayloadContext *data,
+ AVStream *st, AVPacket *pkt, uint32_t *timestamp,
+ const uint8_t *buf, int len, uint16_t seq,
+ int flags)
Instead of "data" you could use "pl_ctx". But this is more or less my
personal opinion.
+{
+ unsigned frame_type;
+ unsigned nr_frames;
+ int err;
+
+ if (len < 2) {
How about a definition like "#define RTP_AC3_PAYLOAD_HEADER_SIZE 2" at
the beginning of the file and use this term here again?
Is it possible that a packet without any AC3 payload is received which
should be forwarded to the decoder anyway?
Otherwise, I would rather check for "len < RTP_AC3_PAYLOAD_HEADER_SIZE + 1".
+ av_log(ctx, AV_LOG_ERROR, "Invalid %d bytes packet\n", len);
+ return AVERROR_INVALIDDATA;
+ }
+
+ frame_type = buf[0] & 0x3;
+ nr_frames = buf[1];
Okay.
+ buf += 2;
+ len -= 2;
RTP_AC3_PAYLOAD_HEADER_SIZE again?
+
+ switch (frame_type) {
+ case 0: /* One or more complete frames */
+ if (nr_frames > 1) {
+ av_log(ctx, AV_LOG_ERROR,
+ "Unimplemented multiple AC3 frames per packet\n");
You could use avpriv_report_missing_feature() here.
+ return AVERROR_PATCHWELCOME;
+ }
+ if (nr_frames == 0) {
+ av_log(ctx, AV_LOG_ERROR, "Invalid AC3 packet data\n");
+ return AVERROR_INVALIDDATA;
+ }
+ if (av_new_packet(pkt, len)) {
+ av_log(ctx, AV_LOG_ERROR, "Out of memory.\n");
+ return AVERROR(ENOMEM);
+ }
+
+ pkt->stream_index = st->index;
+ memcpy(pkt->data, buf, len);
+ return 0;
+
+ case 1:
+ case 2: /* First fragment */
+ free_fragment_if_needed(data);
+
+ data->last_frame = 1;
+ data->nr_frames = nr_frames;
+ err = avio_open_dyn_buf(&data->fragment);
+ if (err < 0)
+ return err;
+
+ avio_write(data->fragment, buf, len);
+ data->timestamp = *timestamp;
+ return AVERROR(EAGAIN);
+
+ case 3: /* Fragment other than first */
+ if (!data->fragment) {
+ av_log(ctx, AV_LOG_WARNING,
+ "Received packet without a start fragment; dropping.\n");
+ return AVERROR(EAGAIN);
+ }
+ if (nr_frames != data->nr_frames ||
+ data->timestamp != *timestamp) {
+ free_fragment_if_needed(data);
+ av_log(ctx, AV_LOG_ERROR, "Invalid packet received\n");
+ return AVERROR_INVALIDDATA;
+ }
+
+ avio_write(data->fragment, buf, len);
+ data->last_frame++;
+ }
+
+ if (!(flags & RTP_FLAG_MARKER))
+ return AVERROR(EAGAIN);
+
+ if (data->last_frame != data->nr_frames) {
+ free_fragment_if_needed(data);
+ av_log(ctx, AV_LOG_ERROR, "Missed %d packets\n",
+ data->nr_frames - data->last_frame);
+ return AVERROR_INVALIDDATA;
+ }
+
+ err = ff_rtp_finalize_packet(pkt, &data->fragment, st->index);
+ if (err < 0) {
+ av_log(ctx, AV_LOG_ERROR,
+ "Error occurred when getting fragment buffer.");
+ return err;
+ }
+
+ return 0;
+}
+
Okay.
+RTPDynamicProtocolHandler ff_ac3_dynamic_handler = {
+ .enc_name = "ac3",
+ .codec_type = AVMEDIA_TYPE_AUDIO,
+ .codec_id = AV_CODEC_ID_AC3,
+ .init = ac3_init,
+ .alloc = ac3_new_context,
+ .free = ac3_free_context,
+ .parse_packet = ac3_handle_packet,
+};
Do you see any use for the optional SDP parameter?
If yes, you could add an AC3 specific parser for SDP line and link it as
".parse_sdp_a_line" line here.
diff --git a/libavformat/rtpdec_formats.h b/libavformat/rtpdec_formats.h
index 803410e..e5aff59 100644
--- a/libavformat/rtpdec_formats.h
+++ b/libavformat/rtpdec_formats.h
@@ -39,6 +39,7 @@ int ff_h263_handle_packet(AVFormatContext *ctx,
PayloadContext *data,
AVStream *st, AVPacket *pkt, uint32_t *timestamp,
const uint8_t *buf, int len, uint16_t seq, int
flags);
+extern RTPDynamicProtocolHandler ff_ac3_dynamic_handler;
extern RTPDynamicProtocolHandler ff_amr_nb_dynamic_handler;
extern RTPDynamicProtocolHandler ff_amr_wb_dynamic_handler;
extern RTPDynamicProtocolHandler ff_g726_16_dynamic_handler;
please add:
- "Changelog" entry
- bump minor / micro version in version.h
- "MAINTAINERS" entry (if you will also be available as maintainer of
your file)
What do you think about a packetizer as counterpart?
Best regards,
Thomas.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
