[FFmpeg-devel] Google Summer of Code participation

[Michael Niedermayer]

On Wed, Apr 08, 2009 at 04:09:01PM +0200, Thilo Borgmann wrote:
>
>
> Michael Niedermayer schrieb:
[...]
>>> Michael Niedermayer schrieb:
>>>     
>>>>> @@ -1586,9 +1586,9 @@ static int audio_decode_frame(VideoState *is, 
>>>>> double *pts_ptr)
>>>>>          /* NOTE: the audio packet can contain several frames */
>>>>>          while (is->audio_pkt_size > 0) {
>>>>>              data_size = sizeof(is->audio_buf1);
>>>>> -            len1 = avcodec_decode_audio2(dec,
>>>>> +            len1 = avcodec_decode_audio3(dec,
>>>>>                                          (int16_t *)is->audio_buf1, 
>>>>> &data_size,
>>>>> -                                        is->audio_pkt_data, 
>>>>> is->audio_pkt_size);
>>>>> +                                        pkt);
>>>>>              if (len1 < 0) {
>>>>>                  /* if error, we skip the frame */
>>>>>                  is->audio_pkt_size = 0;
>>>>>             
>>>> is that the same?
>>>>
>>>>         
>>> Fixed in revision 2.
>>> The last two lines which are altered in ffplay.c (audio_decode_frame):
>>> ::::
>>> -        is->audio_pkt_data = pkt->data;
>>> -        is->audio_pkt_size = pkt->size;
>>> +        avpkt.data = pkt->data;
>>> +        avpkt.size = pkt->size;
>>> ::::
>>> These may have become deprecated now, as the original 
>>> is->audio_pkt_{data,size} are not changed anymore. I may be wrong but I 
>>> think these two lines are for cleaning up. If I'm right, these can be 
>>> deleted now but I'm not 110% sure about that, may be you'll have a look.
>>>     
>>
>> your change to ffplay.c still looks wrong
>>   
> Hm. I compiled it and it works. Forget about the last two lines to be just 
> for cleanup, I understand it right meanwhile...
> I don't see anything wrong in there. The local AVPacket is a dummy packet 
> of which the .data and .size attributes are used as iterating pointers 
> through the real buffer during the while loop.
> After that, if another packet is there to be decoded, this dummy is set to 
> the new packet data.
> So, the 'original' pointer pkt can not be used instead as it has to keep 
> pointing to the begin of the buffer which is free'd after decoding. The 
> local AVPacket is just the new way to provide changeable pointers into the 
> buffer which replaces the use of is->data & is->size.
> Thus, I can't see semantical failure nor unnecessary code... what do you 
> see there which is wrong?

you move variables from the context into local variables, the code does not
look like the values become irrelevant after the function ...


>> also maybe you could split the patch a little the changes to ffplay dont
>> depend on changes to ffmpeg nor apiexample ...
>>
>>   
> Ok I've done although I thought each patch should correlate to one logical 
> entity which I would say that the API changes are... but I don't mind.

patches look ok

[...]


-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090408/28cc9794/attachment.pgp>