Hi, On 30 June 2010 09:53, Martin Storsjö <martin@martin.st> wrote:
On Wed, 30 Jun 2010, Ronald S. Bultje wrote:
On Wed, Jun 30, 2010 at 4:19 AM, Martin Storsjö <martin@martin.st> 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 <rbultje@ronald.bitfreak.net> + */
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