After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 705123 - for some clips, decoder did not get the last frame
for some clips, decoder did not get the last frame
Status: RESOLVED FIXED
Product: gstreamer-vaapi
Classification: Other
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: gstreamer-vaapi maintainer(s)
gstreamer-vaapi maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2013-07-30 06:15 UTC by XuGuangxin
Modified: 2013-08-29 12:57 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
last frame is misssed for this file (17.98 KB, application/octet-stream)
2013-07-30 06:15 UTC, XuGuangxin
  Details
patch to fix this problem (2.49 KB, patch)
2013-07-30 06:36 UTC, XuGuangxin
none Details | Review

Description XuGuangxin 2013-07-30 06:15:41 UTC
Created attachment 250434 [details]
last frame is misssed for this file

There 50 frames in this file.But decoder only output 49 frames.
How to repeat:
1. type:
GST_DEBUG=vaapi*:5 gst-launch-1.0 -v filesrc location=/xxx/CVPA1_TOSHIBA_B.264 ! h264parse ! vaapidecode ! vaapisink
2. count the pop_frame in logs.
3. only 49 output frames.
Comment 1 XuGuangxin 2013-07-30 06:36:43 UTC
Created attachment 250435 [details] [review]
patch to fix this problem

If there are no frame delimiter at the end of stream. You will see this bug.
Root cause:
If the input adapter is emptied. GstVideoDecoder will not call subclass's parser at EOS reached.So last frame is buffered in output adapter.   it never get a chance to send last frame to our decoder.

record bytes count in output adapter can fix this.
Comment 2 Gwenole Beauchesne 2013-08-13 04:32:43 UTC
Hi, actually the real root cause was in the GstVideoDecoder base class implementation. Depending on the stream, it would not submit the expect EOS message.

I have to review your patch again, but it looks like an acceptable workaround.

For instance, could you please test with ffdec_h264 for example, or more precisely avdec_h264 for current 1.0 builds? Thanks. i.e. if both vaapidecode and avdec_h264 miss the last frame, and both are using GstVideoDecoder base class, then this means the issue was in there.

Note: I know gstreamer-libav was ported to the GstVideoDecoder infrastructure, but I have not checked whether this actually was released for GStreamer 1.0.x. Will check later, or you could have a look while doing the test.
Comment 3 XuGuangxin 2013-08-13 05:11:11 UTC
Hi Gwenole:
Yes. I have tried avdec_h264. It's no such problem. you can install gstreamer ppa and take a look.
https://wiki.ubuntu.com/Novacut/GStreamer1.0 
No. Actually, the GstVideoDecoder have accepted EOS. So it called gst_video_decoder_drain_out.
In our case the gst_adapter_available(priv->input_adapter)  will return 0.
Since we take all slices from last frame to output_adapter.We never get a chance to call decoder_class->parse (dec, priv->current_frame,
            priv->input_adapter, TRUE); 




static GstFlowReturn
gst_video_decoder_drain_out (GstVideoDecoder * dec, gboolean at_eos)
{
  GstVideoDecoderClass *decoder_class = GST_VIDEO_DECODER_GET_CLASS (dec);
  GstVideoDecoderPrivate *priv = dec->priv;
  GstFlowReturn ret = GST_FLOW_OK;

  GST_VIDEO_DECODER_STREAM_LOCK (dec);

  if (dec->input_segment.rate > 0.0) {
    /* Forward mode, if unpacketized, give the child class
     * a final chance to flush out packets */
    if (!priv->packetized) {
      while (ret == GST_FLOW_OK && gst_adapter_available (priv->input_adapter)) {
        if (priv->current_frame == NULL)
          priv->current_frame = gst_video_decoder_new_frame (dec);

        ret = decoder_class->parse (dec, priv->current_frame,
            priv->input_adapter, TRUE);
      }
    }
  } else {
    /* Reverse playback mode */
    ret = gst_video_decoder_flush_parse (dec, TRUE);
  }

  if (at_eos) {
    if (decoder_class->finish)
      ret = decoder_class->finish (dec);
  }

  GST_VIDEO_DECODER_STREAM_UNLOCK (dec);

  return ret;
}
Comment 4 Gwenole Beauchesne 2013-08-29 12:56:45 UTC
Fixed in git master branch. Thanks.

Additional changes:
- Renamed bytes_added_to_frame to current_frame_size ;
- Reset current_frame_size to zero on parse error so that we are sure to have valid data when the frame is submitted to the decoder in _finish().
Comment 5 Gwenole Beauchesne 2013-08-29 12:57:12 UTC
commit 111d7d4fa4b2ced74d29d70887b63f2d39cae7d3
Author: Guangxin.Xu <Guangxin.Xu@intel.com>
Date:   Tue Jul 30 14:05:39 2013 +0800

    vaapidecode: submit the last frame from output adapter to decoder.
    
    If there is no frame delimiter at the end of the stream, e.g. no
    end-of-stream or end-of-sequence marker, and that the current frame
    was fully parsed correctly, then assume that last frame is complete
    and submit it to the decoder.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=705123
    
    Signed-off-by: Guangxin.Xu <Guangxin.Xu@intel.com>
    Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>