Quantcast

Is the code in gst_video_encoder_release_frame correct?

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Is the code in gst_video_encoder_release_frame correct?

Martin Maurer
Hi,

the following is an extract from
gst-plugins-base-1.0-1.11.91\gst-libs\gst\video\gstvideoencoder.c

static void
gst_video_encoder_release_frame (GstVideoEncoder * enc,
     GstVideoCodecFrame * frame)
{
   GList *link;

   /* unref once from the list */
   link = g_list_find (enc->priv->frames, frame);
   if (link) {
     gst_video_codec_frame_unref (frame);
     enc->priv->frames = g_list_delete_link (enc->priv->frames, link);
   }
   /* unref because this function takes ownership */
   gst_video_codec_frame_unref (frame);
}

Could there be an error, because the "unref" command is executed twice.
And both time with same parameter "frame".
Is there perhaps a missing return or else statement needed?
Must "frame" in first unref command be "link", or link maybe ok, because
link is the same as frame, in case it is found?

Best regards,

Martin


_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Is the code in gst_video_encoder_release_frame correct?

Sebastian Dröge-3
On Wed, 2017-05-03 at 21:27 +0200, Martin Maurer wrote:

> Hi,
>
> the following is an extract from 
> gst-plugins-base-1.0-1.11.91\gst-libs\gst\video\gstvideoencoder.c
>
> static void
> gst_video_encoder_release_frame (GstVideoEncoder * enc,
>      GstVideoCodecFrame * frame)
> {
>    GList *link;
>
>    /* unref once from the list */
>    link = g_list_find (enc->priv->frames, frame);
>    if (link) {
>      gst_video_codec_frame_unref (frame);
>      enc->priv->frames = g_list_delete_link (enc->priv->frames,
> link);
>    }
>    /* unref because this function takes ownership */
>    gst_video_codec_frame_unref (frame);
> }
>
> Could there be an error, because the "unref" command is executed
> twice. 
> And both time with same parameter "frame".
> Is there perhaps a missing return or else statement needed?
> Must "frame" in first unref command be "link", or link maybe ok,
> because link is the same as frame, in case it is found?
That's correct. One reference is owned by the list, which is unreffed.
The other reference was owned by the caller of release_frame() (from
somewhere the caller must've gotten the reference after all), and that
one is released too here.

--
Sebastian Dröge, Centricular Ltd · http://www.centricular.com
_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

signature.asc (981 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Is the code in gst_video_encoder_release_frame correct?

Yi-Lung Tsai
In reply to this post by Martin Maurer
In gst_video_encoder_chain(), the “frame” is “ref” before appending to frame list.
So ref-count is increased in the list.

gst_video_codec_frame_ref (frame);
priv->frames = g_list_append (priv->frames, frame);

--
Yi-Lung (Bruce) Tsai





On May 4, 2017, at 3:27 AM, Martin Maurer <[hidden email]> wrote:

Hi,

the following is an extract from gst-plugins-base-1.0-1.11.91\gst-libs\gst\video\gstvideoencoder.c

static void
gst_video_encoder_release_frame (GstVideoEncoder * enc,
   GstVideoCodecFrame * frame)
{
 GList *link;

 /* unref once from the list */
 link = g_list_find (enc->priv->frames, frame);
 if (link) {
   gst_video_codec_frame_unref (frame);
   enc->priv->frames = g_list_delete_link (enc->priv->frames, link);
 }
 /* unref because this function takes ownership */
 gst_video_codec_frame_unref (frame);
}

Could there be an error, because the "unref" command is executed twice. And both time with same parameter "frame".
Is there perhaps a missing return or else statement needed?
Must "frame" in first unref command be "link", or link maybe ok, because link is the same as frame, in case it is found?

Best regards,

Martin


_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel


_______________________________________________
gstreamer-devel mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/gstreamer-devel
Loading...