Hi, 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. :-(.
+/** 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.
+ 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].
+ /* frame size code */ + init_get_bits(&gb, buf, len << 3); + config = get_bits(&gb, 3); + switch (config) { + case 0: st->codec->width = 160; st->codec->height = 128 /* FIXME: Wiki says 120? */; break;
Hmm, I guess I should try to generate files with any of these dimensions and see if it uses them then, to get clarity in 128 vs 120. (120 sounds much more natural when combined with 160, though.)
+ case 1: st->codec->width = 128; st->codec->height = 96; break; + 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. Ronald