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

[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

[Yi-Lung Tsai]

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

[hidden email]

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