[FFmpeg-devel] theora depayloader

Josh Allmann joshua.allmann
Sat Mar 20 07:00:03 CET 2010


Third patch attached, comments below:

On 19 March 2010 21:19, Martin Storsj? <martin at martin.st> wrote:
> On Fri, 19 Mar 2010, Josh Allmann wrote:
>
>> + ? ? ? ?// fast first pass to calculate total length
>> + ? ? ? ?for (i = 0, data_len = 0, pkt_len = 0;
>> + ? ? ? ? ? ? i < num_pkts && len > 0;
>> + ? ? ? ? ? ? i++) {
>> + ? ? ? ? ? ?int off = data_len+(i<<1);
>> + ? ? ? ? ? ?pkt_len = AV_RB16(buf+off);
>
> Add some spacing around the operators here, for better readability...

Done, also moved for-loop stuff to the same line.

>
>> + ? ? ? ? ? ?data_len += pkt_len;
>> + ? ? ? ? ? ?len -= pkt_len+2;
>> + ? ? ? }
>> +
>> + ? ? ? if (len < 0){
>> + ? ? ? ? ?av_log(ctx, AV_LOG_ERROR,
>> + ? ? ? ? ? ? ? ? "Length overread by %d\n", -len);
>> + ? ? ? }
>
> Here, you could potentially get the case where len reaches exactly zero,
> before i has reached num_pkts, so we'd get no warning even if the packet
> was invalid...

Hm, good catch. Fixed by checking for len >= 0 in the for loop.

Now that I think of it, a bad packet could potentially give us a large
negative pkt_len, which would in turn make len positive, but still
invalid... so I changed the type declaration of pkt_len (and related
header vars) to uint32_t.

I have not explicitly tested for these conditions, though.

Also added a return AVERROR_INVALIDDATA; I forgot that.

>
>> + ? ? ? if (av_new_packet(pkt, data_len)){
>> + ? ? ? ? ?av_log(ctx, AV_LOG_ERROR, "Out of memory.\n");
>> + ? ? ? ? ?return AVERROR_NOMEM;
>> + ? ? ? }
>> + ? ? ? pkt->stream_index = st->index;
>> +
>> + ? ? ? // concatenate frames
>> + ? ? ? for (i = 0, write_len = 0;
>> + ? ? ? ? ? ?write_len < data_len;
>> + ? ? ? ? ? ?i++){
>> + ? ? ? ? ? pkt_len = AV_RB16(buf);
>> + ? ? ? ? ? buf += 2;
>> + ? ? ? ? ? memcpy(pkt->data+write_len, buf, pkt_len);
>> + ? ? ? ? ? write_len += pkt_len;
>> + ? ? ? ? ? buf += pkt_len;
>> + ? ? ? }
>> + ? ? ? assert(write_len == data_len);
>> +
>> + ? ? ? ?return 0;
>
> Indentation off
>
>> + ? ?}else{
>
> Spaces around the braces
>
>> + ? ? ? ?assert(fragmented < 4);
>> + ? ? ? ?if (data->timestamp != *timestamp){
>> + ? ? ? ? ?// skip if fragmented timestamp is incorrect;
>> + ? ? ? ? ?// a start packet has been lost somewhere
>> + ? ? ? ? ?free_fragment_if_needed(data);
>> + ? ? ? ? ?av_log(ctx, AV_LOG_ERROR, "RTP timestamps don't match!\n");
>> + ? ? ? ? ?return AVERROR_INVALIDDATA;
>> + ? ? ? ?}
>> +
>> + ? ? ? ?// copy data to fragment buffer
>> + ? ? ? ?put_buffer(data->fragment, buf, pkt_len);
>> +
>> + ? ? ? ?if (fragmented == 3) {
>> + ? ? ? ? ? ?// end of theora data packet
>> + ? ? ? ? ? ?uint8_t* theora_data;
>> + ? ? ? ? ? ?int frame_size = url_close_dyn_buf(data->fragment, &theora_data);
>> +
>> + ? ? ? ? ? ?if (frame_size < 0){
>> + ? ? ? ? ? ? ?av_log(ctx, AV_LOG_ERROR, "Error occurred when getting fragment buffer.");
>> + ? ? ? ? ? ? ?return frame_size;
>> + ? ? ? ? ? ?}
>
> Indentation off

Fixed all indentation issues, hopefully for once and for all.

Josh
-------------- next part --------------
A non-text attachment was scrubbed...
Name: theora_depayloader.diff
Type: text/x-patch
Size: 15021 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100320/d5d427a1/attachment.bin>



More information about the ffmpeg-devel mailing list