[Spice-devel] [spice-gtk PATCH 2/2] gst_decoder_queue_frame: free frame when returning false

[Uri Lublin]

On 04/17/2018 03:13 PM, Frediano Ziglio wrote:
>>
>> The decoder_queue_frame now owns frame.
>>
> 
> The queue is named decoding_queue and actually the reason you have to free frame
> is that is not owned by decoding_queue in this path of code.

I was talking about the function not the queue itself.

I'll rewrite the commit log.

> 
> Note that the same issue happens some lines below in the
> 
> #if GST_CHECK_VERSION(1,9,0)
>      if (decoder->appsrc == NULL) {
>          spice_warning("Error: Playbin has not yet initialized the Appsrc element");
>          stream_dropped_frame_on_playback(decoder->base.stream);
>          return TRUE;
>      }
> #endif
> 
> does not matter if the function returns TRUE or FALSE, the caller never free frame
> so either we free or we retain it.

Yes, I missed that return TRUE. I assumed all cases of return TRUE
were already handled.
I'll add it to v2.

Thanks,
     Uri.


> 
>> Signed-off-by: Uri Lublin <uril at redhat.com>
>> ---
>>   src/channel-display-gst.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/channel-display-gst.c b/src/channel-display-gst.c
>> index 0d7aabb..ae59292 100644
>> --- a/src/channel-display-gst.c
>> +++ b/src/channel-display-gst.c
>> @@ -544,6 +544,7 @@ static gboolean
>> spice_gst_decoder_queue_frame(VideoDecoder *video_decoder,
>>       if (decoder->pipeline == NULL) {
>>           /* An error occurred, causing the GStreamer pipeline to be freed */
>>           spice_warning("An error occurred, stopping the video stream");
>> +        frame->free(frame);
>>           return FALSE;
>>       }
>>   
> 
> Frediano
>