Thanks for taking the time to report this.

Can you provide a sample file / test case? Because all the mpeg2 sample files I have found in the net, mpegpsdemux forbids the seek operation.

Mind I just tested my sample and without the fix in https://bugzilla.gnome.org/show_bug.cgi?id=749604 applied beforehand totem segfaults at video playback start (on mpeg2 too).

About the samples : mine http://prahal.homelinux.net/~prahal/gstreamer/tests/0/M2U01617.MPG or others http://www.snapstream.com/enterprise/samples.asp
downloaded locally then played in totem  with gstreamer-vaapi installed and seek backward or forward segfaults.


I am on gstreamer and its plugins from debian experimental ie 1.5.0.1+git20150513-1 + gstreamer-vaapi git master + the mentioned fix (749604 and current bug report ones).

I just build totem and I confirm: it crashes.

But there's an interesting issue: if you run these MPEG2 samples with gst-play-1.0 it rejects the seek operation, but, of course, it is possible with other media.

In gst-play.c

  gst_query_parse_seeking (query, NULL, &seekable, NULL, &dur);
  gst_query_unref (query);

  if (!seekable || dur <= 0)
    goto seek_failed;


In bacon-video-widget.c

    query = gst_query_new_seeking (GST_FORMAT_TIME);
    if (gst_element_query (bvw->priv->play, query)) {
      gst_query_parse_seeking (query, NULL, &res, NULL, NULL);
      GST_DEBUG ("seeking query says the stream is%s seekable", (res) ? "" : " not");
      bvw->priv->seekable = (res) ? 1 : 0;
    } else {
      GST_DEBUG ("seeking query failed");
    }
    gst_query_unref (query);
  }


For the gst-play, a stream is seekable only if the segment's end is defined (not negative), meanwhile, in totem, the seekable segment is ignored.

I need to understand what's the correct  approach. If MPEG2 is supposed not to be seekable, the current gstreamer-vaapi code is correct. Otherwise, gst-play.c should be fixed too.

Hi, please try the following patch instead. I'd favour the fixes from Jan, which looks more correct to me. Thanks.

https://github.com/thaytan/gstreamer-vaapi/commit/b84e65ada15f9396b8c9ab507c1cb5f10c3f3914

(In reply to Gwenole Beauchesne from comment #5)
> Hi, please try the following patch instead. I'd favour the fixes from Jan,
> which looks more correct to me. Thanks.
> 
> https://github.com/thaytan/gstreamer-vaapi/commit/
> b84e65ada15f9396b8c9ab507c1cb5f10c3f3914

I've just tried them. There's no crash anymore, but the decoder halts:

(totem:6000): GStreamer-CRITICAL **: _gst_util_uint64_scale: assertion 'denom != 0' failed

(totem:6000): GStreamer-CRITICAL **: _gst_util_uint64_scale: assertion 'denom != 0' failed
0:00:04.794673003  6000       0x68f810 ERROR                default totem-gst-helpers.c:61:totem_gst_message_print: message = No valid frames decoded before end of stream
0:00:04.794726937  6000       0x68f810 ERROR                default totem-gst-helpers.c:63:totem_gst_message_print: domain  = 5004 (gst-stream-error-quark)
0:00:04.794735915  6000       0x68f810 ERROR                default totem-gst-helpers.c:64:totem_gst_message_print: code    = 7
0:00:04.794741558  6000       0x68f810 ERROR                default totem-gst-helpers.c:65:totem_gst_message_print: debug   = gstvideodecoder.c(1192): gboolean gst_video_decoder_sink_event_default(GstVideoDecoder *, GstEvent *) (): /GstPlayBin:play/GstURIDecodeBin:uridecodebin0/GstDecodeBin:decodebin0/GstVaapiDecodeBin:vaapidecodebin0/GstVaapiDecode:vaapidecode:
no valid frames found
0:00:04.794750861  6000       0x68f810 ERROR                default totem-gst-helpers.c:66:totem_gst_message_print: source  = <vaapidecode>
0:00:04.794758491  6000       0x68f810 ERROR                default totem-gst-helpers.c:67:totem_gst_message_print: uri     = (NULL)

Here it also works.

That is above gstreamer-vaapi  f304d4168b3d5d878a43bca595adf37ef446b420 , bug 749604 fix (for gstreamer-vaapi + cogl crasher, here 1.18) .


I reverted my fix  and added  https://github.com/thaytan/gstreamer-vaapi/commit/
 b84e65ada15f9396b8c9ab507c1cb5f10c3f3914 .

The "No valid frames decoded before end of stream" happens to me a few times. Seems my test file showcase this issue. You could check by only pressing backward . This should not error out.

The denom != 0 I have not reproduced.

Cool.

Just found that running in debug vaapidecode:5 it crashes again. The diff fixes that:

--- a/gst-libs/gst/vaapi/gstvaapidecoder_mpeg2.c
+++ b/gst-libs/gst/vaapi/gstvaapidecoder_mpeg2.c
@@ -1141,8 +1141,6 @@ decode_slice(GstVaapiDecoderMpeg2 *decoder, GstVaapiDecoderUnit *unit)
         GST_VAAPI_DECODER_CODEC_FRAME(decoder)->input_buffer;
     GstMapInfo map_info;
 
-    GST_DEBUG("slice %d (%u bytes)", slice_hdr->mb_row, unit->size);
-
     if (!is_valid_state(decoder, GST_MPEG_VIDEO_STATE_VALID_PIC_HEADERS))
         return GST_VAAPI_DECODER_STATUS_SUCCESS;
 
@@ -1151,6 +1149,8 @@ decode_slice(GstVaapiDecoderMpeg2 *decoder, GstVaapiDecoderUnit *unit)
         return GST_VAAPI_DECODER_STATUS_ERROR_UNKNOWN;
     }
 
+    GST_DEBUG("slice %d (%u bytes)", slice_hdr->mb_row, unit->size);
+
     slice = GST_VAAPI_SLICE_NEW(MPEG2, decoder,
         (map_info.data + unit->offset), unit->size);
     gst_buffer_unmap(buffer, &map_info);

(In reply to Alban Browaeys from comment #7)
> That is above gstreamer-vaapi  f304d4168b3d5d878a43bca595adf37ef446b420 ,
> bug 749604 fix (for gstreamer-vaapi + cogl crasher, here 1.18) .

I don't understand.

Are you saying the commit f304d41 fixes bug #749604 ???

That's the last commit in gitorious. Are you aware that the official repository has moved to github??

https://github.com/01org/gstreamer-vaapi

Sorry. I was saying b84e65ada fixed the issue here (and that my master was at  f304d41  and indeed I failed to see the switch to github so it is gitorious that I am at). 

Let me switch to github .

Now on github. Issue reproduced. Patch reapplied (commit b84e65ada15f9) and it fixes the seek in mpeg2. Thanks

gstreamer-vaapi master at c393b7e2ab23c96f9a67ed2966083cda5034328a github .

These commits were pushed:

commit d6255be9398f5d6803c0997446a4b89c10beab26
Author: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
Date:   Wed May 27 10:49:56 2015 +0200

    mpeg2: avoid crash when seeking with debug logs
    
    Move down the debug message when the state of the decoder is verified
    so the slice header is not NULL.

commit f1a60ec6a2d9523c10713a43871df22ddc68d720
Author: Jan Schmidt <jan@centricular.com>
Date:   Wed Dec 17 00:41:10 2014 +1100

    mpeg2: Avoid crashes and warnings on re-opened decoder after a seek
    
    Reset state and add some checks for safe state to avoid a crash and
    a warning after the decoder is destroyed/recreated during a seek.

(In reply to Víctor Manuel Jáquez Leal from comment #8)
> Cool.
> 
> Just found that running in debug vaapidecode:5 it crashes again. The diff
> fixes that:
> 
> --- a/gst-libs/gst/vaapi/gstvaapidecoder_mpeg2.c
> +++ b/gst-libs/gst/vaapi/gstvaapidecoder_mpeg2.c
> @@ -1141,8 +1141,6 @@ decode_slice(GstVaapiDecoderMpeg2 *decoder,
> GstVaapiDecoderUnit *unit)
>          GST_VAAPI_DECODER_CODEC_FRAME(decoder)->input_buffer;
>      GstMapInfo map_info;
>  
> -    GST_DEBUG("slice %d (%u bytes)", slice_hdr->mb_row, unit->size);
> -
>      if (!is_valid_state(decoder, GST_MPEG_VIDEO_STATE_VALID_PIC_HEADERS))
>          return GST_VAAPI_DECODER_STATUS_SUCCESS;
>  
> @@ -1151,6 +1149,8 @@ decode_slice(GstVaapiDecoderMpeg2 *decoder,
> GstVaapiDecoderUnit *unit)
>          return GST_VAAPI_DECODER_STATUS_ERROR_UNKNOWN;
>      }
>  
> +    GST_DEBUG("slice %d (%u bytes)", slice_hdr->mb_row, unit->size);
> +
>      slice = GST_VAAPI_SLICE_NEW(MPEG2, decoder,
>          (map_info.data + unit->offset), unit->size);
>      gst_buffer_unmap(buffer, &map_info);

It would be better to know why slice_hdr or unit is NULL. Committing this is just hiding the core issue more...