Hi,

On 30 June 2010 09:53, Martin Storsjö <[email protected]> wrote:
> On Wed, 30 Jun 2010, Ronald S. Bultje wrote:
>
>> On Wed, Jun 30, 2010 at 4:19 AM, Martin Storsjö <[email protected]> wrote:
>> > On Wed, 30 Jun 2010, Josh Allmann wrote:
>> >> +/**
>> >> + * @file libavformat/rtpdec_svq3.c
>> >> + * @brief RTP support for the SV3V (SVQ3) payload
>> >> + * @author Ronald S. Bultje <[email protected]>
>> >> + */
>> >
>> > Lately, we've been instructed to remove the file names from these @file
>> > directives, and just keep "@file". Also, it would be good to mention which
>> > RFC this implements, to make it easier for the reader to look up the
>> > details.
>>
>> No RFC, this is RE'ed stuff. :-(.
>
> I figured that out after googling a bit, but to save me from googling next
> time, it may be good to add a notice that there's no RFC.
>

Removed the filename from the @file directive, and added a note
indicating this depacketizer is RE'ed.

>> >> +/** return 0 on packet, <1 on partial packet or error... */
>> >
>> > Isn't this comment a bit wrong? Iirc, return 0 on packet, 1 on partial
>> > packets (that is, more packets can be returned), <0 on error. When
>> > checking other depacketizers, others seem to have some kind of confusion,
>> > too.
>>
>> Depends on spreading. We return 1 if the RTP packet contained multiple
>> demuxer packets, e.g. two video frames. What happens here is that >=2
>> RTP packets contain 1 video frame, so we don't have any data after 1
>> RTP packet (partial packet) and thus return -1.
>
> Ah, yeah, well even when referring to that, the comment which says "<1 on
> partial packet or error" should be "<0" of course.
>

Yeah, my bad; the original comment Ronald wrote was correct and I
screwed it up while editing.

>> >> +    config_packet = buf[0] & 0x40;
>> >> +    start_packet  = buf[0] & 0x20;
>> >> +    end_packet    = buf[0] & 0x10;
>> >> +    buf += 2;
>> >> +    len -= 2;
>> >
>> > What's in buf[1], that we apparently ignore? :-) I'm not sure if a comment
>> > describing it here is necessary, a RFC reference somewhere would be enough
>> > so that I'd find it easily.
>>
>> buf[1] is probably padding for future flags extension, which of course
>> never happened. The binary didn't use buf[1].
>
> Ok, if there's no RFC, this is a very good explanation indeed. :-)

Comment added indicating this.

>
>> >> +            case 2: st->codec->width = 176; st->codec->height = 144; 
>> >> break;
>> >> +            case 3: st->codec->width = 352; st->codec->height = 288; 
>> >> break;
>> >> +            case 4: st->codec->width = 704; st->codec->height = 576; 
>> >> break;
>> >> +            case 5: st->codec->width = 240; st->codec->height = 180; 
>> >> break;
>> >> +            case 6: st->codec->width = 320; st->codec->height = 240; 
>> >> break;
>> >> +            case 7: /**< in case of '111', the next 2x12 bits carry 
>> >> optional width/height */
>> >> +                st->codec->width  = get_bits(&gb, 12);
>> >> +                st->codec->height = get_bits(&gb, 12);
>> >> +                break;
>>
>> This whole block can be removed (I removed it locally later, but never
>> resubmitted), because the decoder does this for us.
>
> Yeah, I was about to suggest that, too.
>

Removed, but there's still the same problem as QDM2 -- extradata in
RTP rather than SDP -- so to delay decoder initialization, I used the
CODEC_ID_NONE trick that Ronald proposed.

Tested on all the SVQ3 samples that Martin provided. Works well, and I
didn't get any "slice after bitstream end" errors. Those were probably
artifacts from the slight broken-ness of the last patch.

Also attached is a patch that adds FF_INPUT_BUFFER_PADDING_SIZE zero
bytes to every url_close_dyn_buf call.

QDM2 patch forthcoming; I'll make a new thread for that.

Josh
From 50f23572a672ee51b9728b44ef2375d478b57c2e Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Wed, 30 Jun 2010 19:06:27 -0700
Subject: [PATCH 1/2] Add RTP depacketization of SVQ3.

---
 Changelog                 |    1 +
 libavformat/Makefile      |    1 +
 libavformat/rtpdec.c      |    2 +
 libavformat/rtpdec_svq3.c |  131 +++++++++++++++++++++++++++++++++++++++++++++
 libavformat/rtpdec_svq3.h |   33 +++++++++++
 5 files changed, 168 insertions(+), 0 deletions(-)
 create mode 100644 libavformat/rtpdec_svq3.c
 create mode 100644 libavformat/rtpdec_svq3.h

diff --git a/Changelog b/Changelog
index be3b074..49990be 100644
--- a/Changelog
+++ b/Changelog
@@ -16,6 +16,7 @@ version <next>:
 - DTS-ES extension (XCh) decoding support
 - native VP8 decoder
 - RTSP tunneling over HTTP
+- RTP depacketization of SVQ3
 
 
 
diff --git a/libavformat/Makefile b/libavformat/Makefile
index 2977673..5291fe3 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -229,6 +229,7 @@ OBJS-$(CONFIG_SDP_DEMUXER)               += rtsp.o        \
                                             rtpdec_h263.o \
                                             rtpdec_h264.o \
                                             rtpdec_mpeg4.o \
+                                            rtpdec_svq3.o \
                                             rtpdec_xiph.o
 OBJS-$(CONFIG_SEGAFILM_DEMUXER)          += segafilm.o
 OBJS-$(CONFIG_SHORTEN_DEMUXER)           += raw.o id3v2.o
diff --git a/libavformat/rtpdec.c b/libavformat/rtpdec.c
index 38a4c5b..ca06bff 100644
--- a/libavformat/rtpdec.c
+++ b/libavformat/rtpdec.c
@@ -35,6 +35,7 @@
 #include "rtpdec_h263.h"
 #include "rtpdec_h264.h"
 #include "rtpdec_mpeg4.h"
+#include "rtpdec_svq3.h"
 #include "rtpdec_xiph.h"
 
 //#define DEBUG
@@ -68,6 +69,7 @@ void av_register_rtp_dynamic_payload_handlers(void)
     ff_register_dynamic_payload_handler(&ff_h264_dynamic_handler);
     ff_register_dynamic_payload_handler(&ff_vorbis_dynamic_handler);
     ff_register_dynamic_payload_handler(&ff_theora_dynamic_handler);
+    ff_register_dynamic_payload_handler(&ff_svq3_dynamic_handler);
 
     ff_register_dynamic_payload_handler(&ff_ms_rtp_asf_pfv_handler);
     ff_register_dynamic_payload_handler(&ff_ms_rtp_asf_pfa_handler);
diff --git a/libavformat/rtpdec_svq3.c b/libavformat/rtpdec_svq3.c
new file mode 100644
index 0000000..379ec6c
--- /dev/null
+++ b/libavformat/rtpdec_svq3.c
@@ -0,0 +1,131 @@
+/*
+ * Sorenson-3 (SVQ3/SV3V) payload for RTP
+ * Copyright (c) 2010 Ronald S. Bultje
+ *
+ * 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
+ */
+
+/**
+ * @file
+ * @brief RTP support for the SV3V (SVQ3) payload (RE'ed)
+ * @author Ronald S. Bultje <[email protected]>
+ */
+
+#include <string.h>
+#include <libavcodec/get_bits.h>
+#include "rtp.h"
+#include "rtpdec.h"
+#include "rtpdec_svq3.h"
+
+struct PayloadContext {
+    ByteIOContext *pktbuf;
+    int64_t        timestamp;
+    int            is_keyframe;
+};
+
+/** return 0 on packet, <0 on partial packet or error... */
+static int sv3v_parse_packet (AVFormatContext *s, PayloadContext *sv,
+                              AVStream *st, AVPacket *pkt,
+                              uint32_t *timestamp,
+                              const uint8_t *buf, int len, int flags)
+{
+    int config_packet, start_packet, end_packet;
+
+    if (len < 2)
+        return AVERROR_INVALIDDATA;
+
+    config_packet = buf[0] & 0x40;
+    start_packet  = buf[0] & 0x20;
+    end_packet    = buf[0] & 0x10;
+    buf += 2;     // ignore buf[1]
+    len -= 2;
+
+    if (config_packet) {
+
+        if (sv->timestamp)
+            av_free(st->codec->extradata);
+        if (len < 2 || !(st->codec->extradata = av_malloc(len + 8 + FF_INPUT_BUFFER_PADDING_SIZE)))
+            return AVERROR_INVALIDDATA;
+
+        st->codec->extradata_size = len + 8;
+        memcpy(st->codec->extradata, "SEQH", 4);
+        AV_WB32(st->codec->extradata + 4, len);
+        memcpy(st->codec->extradata + 8, buf, len);
+        st->codec->codec_id = CODEC_ID_SVQ3;
+
+        return AVERROR(EAGAIN);
+    }
+
+    if (start_packet) {
+        int res;
+
+        if (sv->pktbuf) {
+            uint8_t *tmp;
+            url_close_dyn_buf(sv->pktbuf, &tmp);
+            av_free(tmp);
+        }
+        if ((res = url_open_dyn_buf(&sv->pktbuf)) < 0)
+            return res;
+        sv->timestamp   = *timestamp;
+        sv->is_keyframe = flags & RTP_FLAG_KEY;
+    }
+
+    if (!sv->pktbuf)
+        return AVERROR_INVALIDDATA;
+
+    put_buffer(sv->pktbuf, buf, len);
+
+    if (end_packet) {
+        av_init_packet(pkt);
+        pkt->stream_index = st->index;
+        pkt->pts          = sv->timestamp;
+        pkt->flags        = sv->is_keyframe ? AV_PKT_FLAG_KEY : 0;
+        pkt->size         = url_close_dyn_buf(sv->pktbuf, &pkt->data);
+        pkt->destruct     = av_destruct_packet;
+        sv->pktbuf        = NULL;
+        return 0;
+    }
+
+    return AVERROR(EAGAIN);
+}
+
+static PayloadContext *
+sv3v_extradata_new(void)
+{
+    return av_mallocz(sizeof(PayloadContext));
+}
+
+static void
+sv3v_extradata_free(PayloadContext *sv)
+{
+    if (sv->pktbuf) {
+        uint8_t *buf;
+        url_close_dyn_buf(sv->pktbuf, &buf);
+        av_free(buf);
+    }
+    av_free(sv);
+}
+
+RTPDynamicProtocolHandler ff_svq3_dynamic_handler = {
+    "X-SV3V-ES",
+    CODEC_TYPE_VIDEO,
+    CODEC_ID_NONE,
+    NULL,                   // parse sdp line
+    sv3v_extradata_new,
+    sv3v_extradata_free,
+    sv3v_parse_packet,
+};
diff --git a/libavformat/rtpdec_svq3.h b/libavformat/rtpdec_svq3.h
new file mode 100644
index 0000000..267d4d9
--- /dev/null
+++ b/libavformat/rtpdec_svq3.h
@@ -0,0 +1,33 @@
+/*
+ * Sorenson-3 (SVQ3/SV3V) payload for RTP
+ * Copyright (c) 2010 Ronald S. Bultje
+ *
+ * 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
+ */
+
+#ifndef AVFORMAT_RTPDEC_SVQ3_H
+#define AVFORMAT_RTPDEC_SVQ3_H
+
+#include "libavcodec/avcodec.h"
+#include "rtpdec.h"
+
+/**
+ * Sorenson-3 RTP callbacks.
+ */
+extern RTPDynamicProtocolHandler ff_svq3_dynamic_handler;
+
+#endif /* AVFORMAT_RTPDEC_SVQ3_H */
-- 
1.7.0.4

From 9dd82945900db6d3283661936b84a594bf6524c6 Mon Sep 17 00:00:00 2001
From: Josh Allmann <[email protected]>
Date: Wed, 30 Jun 2010 19:56:59 -0700
Subject: [PATCH 2/2] Add padding to every url_close_dyn_buf call.

---
 libavformat/aviobuf.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 8684903..8225639 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -893,7 +893,10 @@ int url_open_dyn_packet_buf(ByteIOContext **s, int max_packet_size)
 int url_close_dyn_buf(ByteIOContext *s, uint8_t **pbuffer)
 {
     DynBuffer *d = s->opaque;
-    int size;
+    int size, i;
+
+    for (i = 0; i < FF_INPUT_BUFFER_PADDING_SIZE; i++)
+        put_byte(s, 0);
 
     put_flush_packet(s);
 
-- 
1.7.0.4

_______________________________________________
FFmpeg-soc mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc

Reply via email to