[FFmpeg-devel] Google Summer of Code participation

[Thilo Borgmann]

Michael Niedermayer schrieb:
> On Tue, Apr 07, 2009 at 11:32:36PM +0200, Thilo Borgmann wrote:
>   
>> Ronald S. Bultje schrieb:
>>     
>>>   
>>>       
>>>> But I think it would make sense to create a new function
>>>> "try_decode_frame2()" in the same way as for avcodec_decode_video() 
>>>> because
>>>> it is parameterized with a pointer to the buffer and an int for the size,
>>>> but it should be called using an AVPacket struct instead. What do you 
>>>> think?
>>>>     
>>>>         
>>> Michael may be better at answering here, but it's static to utils.c,
>>> so no need for a new function, just change the prototype and update
>>> the callers accordingly.
>>>
>>>
>>>   
>>>       
>> Ok, I got the api-example.c, ffplay.c and ffmpeg.c redone for revision 1 of 
>> the api changes patch.
>> As it is static seems to be a good reason not go the long way, ok.
>>
>> Revision 1 attached.
>>     
>
> [...]
>   
>> @@ -1184,29 +1183,32 @@ static int output_packet(AVInputStream *ist, int ist_index,
>>      static short *samples= NULL;
>>      AVSubtitle subtitle, *subtitle_to_free;
>>      int got_subtitle;
>> +    AVPacket avpkt;
>> +
>> +    av_init_packet(&avpkt);
>>  
>>      if(ist->next_pts == AV_NOPTS_VALUE)
>>          ist->next_pts= ist->pts;
>>  
>>      if (pkt == NULL) {
>>          /* EOF handling */
>> -        ptr = NULL;
>> -        len = 0;
>> +        avpkt.data = NULL;
>> +        avpkt.size = 0;
>>          goto handle_eof;
>>     
>
> what values do data & size have prior to these assignments?
>   
Undefined. Since they might be used for calls to the decoder funtions 
from within the following switch, they need to be well defined - just 
like their predecessors ptr and len.
Is there something I don't understand?
>
> [...]
>   
>> @@ -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?
>
>   
Shame on me, trapped again. Ok, but since my idea to do it the right way 
would be to create a local AVPacket of which the date & size members are 
to be changed like is->audio_pkt_data & is->audio_pkt_size are....
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index 54c9202..1b89552 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -1847,6 +1847,11 @@ static int try_decode_frame(AVStream *st, const uint8_t *data, int size)
>>      AVCodec *codec;
>>      int got_picture, data_size, ret=0;
>>      AVFrame picture;
>> +    AVPacket avpkt;
>> +
>> +    av_init_packet(&avpkt);
>> +    avpkt.data = data;
>> +    avpkt.size = size;
>>  
>>    if(!st->codec->codec){
>>      codec = avcodec_find_decoder(st->codec->codec_id);
>> @@ -1860,16 +1865,16 @@ static int try_decode_frame(AVStream *st, const uint8_t *data, int size)
>>    if(!has_codec_parameters(st->codec)){
>>      switch(st->codec->codec_type) {
>>      case CODEC_TYPE_VIDEO:
>> -        ret = avcodec_decode_video(st->codec, &picture,
>> -                                   &got_picture, data, size);
>> +        ret = avcodec_decode_video2(st->codec, &picture,
>> +                                   &got_picture, &avpkt);
>>          break;
>>      case CODEC_TYPE_AUDIO:
>> -        data_size = FFMAX(size, AVCODEC_MAX_AUDIO_FRAME_SIZE);
>> +        data_size = FFMAX(avpkt.size, AVCODEC_MAX_AUDIO_FRAME_SIZE);
>>          samples = av_malloc(data_size);
>>          if (!samples)
>>              goto fail;
>> -        ret = avcodec_decode_audio2(st->codec, samples,
>> -                                    &data_size, data, size);
>> +        ret = avcodec_decode_audio3(st->codec, samples,
>> +                                    &data_size, &avpkt);
>>          av_free(samples);
>>          break;
>>      default:
>>     
>
> a mess
>
>   
... what I already did here and you obviously don't like it,  in which 
way would you think this should be done? Or maybe, I don't get your 
thoughts right... please tell me.

TB