[FFmpeg-devel] [PATCH] RTP/QDM2 parser

[Ronald S. Bultje]

Hi,

On Fri, Aug 7, 2009 at 11:19 AM, Reimar
D?ffinger<Reimar.Doeffinger at gmx.de> wrote:
> I have some nit-picks if you don't mind.

Much appreciated. I'll work on the comments you gave. Here's a few
quick replies:

> On Tue, Aug 04, 2009 at 05:15:10PM -0400, Ronald S. Bultje wrote:
>> + ? ?if ((res = av_new_packet(pkt, qdm->block_size)) < 0)
>> + ? ? ? ?return res;
[..]
>> + ? ?memset(pkt->data, 0, pkt->size);
>
> Hm. While at it you might consider setting the padding to 0, too...

av_new_packet() does that already.

>> + ? ? ? ?if (++qdm->n_pkts < qdm->subpkts_per_block)
>> + ? ? ? ? ? ?return -1;
>
> Nothing useful you can do with partial packets with little extra effort?

Will do, but in a separate patch, if that's OK. It adds a little
complexity that I'd like to avoid for the first submission.

>> +static PayloadContext *qdm2_extradata_new(void)
>> +{
>> + ? ?return av_mallocz(sizeof(PayloadContext));
>
> This looks wrong. Sizes of structs can depend on how the compiler
> packs them, I don't think extradata should ever depend on the compiler
> used...

I made the same "mistake" on SV3V, it should read "context", not
extradata, my patches are based on antique versions of rtp_*.c where
we named everything "extradata", this was renamed to "context"
recently but my patches are old. Will fix.

Ronald