I have been playing with changing the playback rate. Also posted a patch for adding that support in gst-play (bug 745174).

And yes, most software decoders support the feature.

vaapidecoder supports the increase of the playback rate and the slow down, but not the reverse. The current behavior is just to stuck and freeze in the last image.

Created attachment 298109 [details]
Reverse playback backtrace

gdb backtrace of a video is tried to be played in reverse.

The interesting part of the uploaded backtrace (attachment 298109 [details]) is 

+ Trace 234728

Created attachment 298637 [details] [review]
vaapidecode: handle flush() vmethod

Since GStreamer 1.2 the vmethod reset() in GstVideoDecoderClass was deprecated
and flush() was added.

This patch set the method flush if the installed GStreamer version is 1.2 or
superior. Otherwise, reset() is set.

Created attachment 298638 [details] [review]
vaapidecode: drop queued frames when flush()

The flush operation intent to discard all remaining data from the decoder
without pushing it downstream. This implies discard all the queued frames in
the decoder.

This patch drop all the frames in processing queue. This makes, in general,
the seek operation faster. In the case of playback rate increasing or
decreasing, the first frame to play will depend on the demuxer, because some
will repeat some buffers already played.

The vmethod finish() is modified to use the internal flush, not the new vmethod.

The method reset_full() is modified to use the new vmethod.

Created attachment 298639 [details] [review]
vaapidecode: add drain() vmethod

In GStremer v1.6 a new vmethod drain() was added in GstVideoDecoder
class. This patch implements this new method.

With these patches, when playing reversed the vaapidecode thread doesn't get blocked anymore, but still it's not working.

As far I as I understand, the problem may be because vaapidecode is not a packetized decoder, and perhaps that use case, in GstVideoDecoder, is not well tested yet.

Review: 298637 
Review: 298638 

There could be issues if you avoid the reset_full() code path while doing seeking.
Other than the flushing of queued buffers, we have to reset many of the internal state variables(in core libgstvaapi) too, which is not handling by the current code. That was the reason to destroy()/create() decoder for each seek.

I agree that we need to fix the flush() with the correct behavior as you mentioned. But it requires careful changes in core libgstvaapi and some major testings too. Actually this was in my to-do list, but didn't get time to look into :)..

IMHO, it is better to stick on with the current behavior for now, which means follow the reset_full() for each seek ,so that we will be in a safe side even though there is a disadvantage of slow-seek(that might not be a big deal??). 
And fix the reverse playback issue. Once it get fixed,
We can create another bug report for optimizations like "don't destroy and create decoder for each seek" or something.

Also, I'm wondering if I should keep the conditional compilation given the bug #745728. I guess it would be better just remove the vmethod reset() and don't clutter vaapidecode.

Created attachment 298977 [details] [review]
vaapidecode: handle flush() vmethod

Since GStreamer 1.2 the vmethod reset() in GstVideoDecoderClass was deprecated
and flush() was added.

This patch set the vmethod flush() if the installed GStreamer version is 1.2 or
superior. Otherwise, reset() is set.

v2: 1) In order to avoid symbol collision, the old method gst_vaapidecode_flush()
       was renamed to gst_vaapidecode_internal_flush().
    2) The new vmethod flush() always do a hard full reset.

Created attachment 298978 [details] [review]
vaapidecode: remove vmethod reset()

Since in bug #745728 the support for GStreamer 1.0 is going to be dropped,
this patch removes the method reset() which was deprecated in GStreamer 1.2.

Created attachment 298979 [details] [review]
vaapidecode: add drain() vmethod

In GStremer v1.6 a new vmethod drain() was added in GstVideoDecoder
class. This patch implements this new method.

Could you please check whether your patches satisfying the cases explained in 
commit a6436f27d5103cf01e180f1100a9ccc8a6bbaa87

In my first look, it seems that you have removed couples of stuffs from flush routine.

Created attachment 299086 [details] [review]
vaapidecode: handle flush() vmethod

Since GStreamer 1.2 the vmethod reset() in GstVideoDecoderClass was deprecated
and flush() was added.

This patch set the vmethod flush() if the installed GStreamer version is 1.2 or
superior. Otherwise, reset() is set.

v2: 1) In order to avoid symbol collision, the old method gst_vaapidecode_flush()
       was renamed to gst_vaapidecode_internal_flush().
    2) The new vmethod flush() always do a hard full reset.
v3: 1) Call gst_vaapidecode_internal_flush() first in flush() vmethod, in order to
       gather all collected data with  gst_video_decoder_have_frame()

Created attachment 299087 [details] [review]
vaapidecode: remove vmethod reset()

Since in bug #745728 the support for GStreamer 1.0 is going to be dropped,
this patch removes the method reset() which was deprecated in GStreamer 1.2.

Created attachment 299088 [details] [review]
vaapidecode: add drain() vmethod

In GStremer v1.6 a new vmethod drain() was added in GstVideoDecoder
class. This patch implements this new method.

Review of attachment 299086 [details] [review]:

Pushed,
commit 1bd810fe99a29c869230e69ff31730e92bec9f33
Author: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>
Date:   Mon Mar 16 23:36:33 2015 +0200

    vaapidecode: handle flush() vmethod

Review of attachment 299087 [details] [review]:

Pushed, Thanks for the patch.
commit 49606b8d2547e7ea1d7e805f9b55b30e821f7af5
Author: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>
Date:   Mon Mar 16 23:37:29 2015 +0200

    vaapidecode: remove vmethod reset()

Review of attachment 299088 [details] [review]:

Pushed, Thanks for the patch
commit 71d91c77163036ee30d5aac68b7c3797bba943a8
Author: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>
Date:   Mon Mar 16 23:38:18 2015 +0200

    vaapidecode: add drain() vmethod

After debugging a lot and talking with Sebastian, I see that reverse playback with non-packetized decoders (those which do their own parsing, such as gstreamver-vaapi) is not supported by GstVideoDecoder: it is based on having whole decoded frames in parse_gather list, which is not done since the non-packetized just push the buffers into the adapter:

  if (priv->packetized) {
    if (!GST_BUFFER_FLAG_IS_SET (buf, GST_BUFFER_FLAG_DELTA_UNIT)) {
      GST_VIDEO_CODEC_FRAME_SET_SYNC_POINT (priv->current_frame);
    }

    priv->current_frame->input_buffer = buf;

    if (decoder->input_segment.rate < 0.0) {
      priv->parse_gather =
          g_list_prepend (priv->parse_gather, priv->current_frame);
    } else {
      ret = gst_video_decoder_decode_frame (decoder, priv->current_frame);
    }
    priv->current_frame = NULL;
  } else {
    gst_adapter_push (priv->input_adapter, buf);

    ret = gst_video_decoder_parse_available (decoder, at_eos, TRUE);
  }


Somehow we need to append the decoded frames to the parse_gather in the non-packetized branch

With the patches in bug 747574, I could make the reverse playback to work, but its logic is not friendly with decoders that have a limited number of output buffers (in our case surfaces):

The reverse playback in GstVideoDecoder, if I understand it correctly, gathers a bunch of frames, traverses the list backwards, decoding each frame; finally pushes the whole list to the next element.

The problem is that the list of frames to decoder is bigger that our number of available surfaces. Hence, there's a moment where the decoder run out of surfaces leading to a blocking condition, but the reversed frame list is not decoded completely. Race condition.

We could change the logic of reverse playback in GstVideoDecoder, pushing every frame after its decoding, but I don't know if that is correct.

Created attachment 310636 [details]
vaapidecode Debug messages

Since it is related, I am commenting here too

I have applied the patches 227 to 230 to gstreamer-plugin-base version 1.4.3. Moreover I have realized that, the patches 299086, 299087 and 299088 in <a href="">Bug 742922</a> are pushed in gstreamer-vaapi version 0.6.0.

But the reverse playback is still not working. What should I do?

One can find the output of element (GST_DEBUG=vaapidecode:7) when a -1.0 seek_event is sent to pipeline: <a href"https://bugzilla.gnome.org/attachment.cgi?id=310636">Attachment 310636 [details]</a>

Regards.

(In reply to Engin FIRAT from comment #23)
> But the reverse playback is still not working. What should I do?

Thanks for your interest. As you can see, this bug is not closed yet, but it is not completed yet either.

There's a problem in the reverse playback in the video decoder base class: it doesn't consider that the decoder might have a limited number of output buffers when gathering the GOP. That's is why vaapidecode stalls when going to reverse playback.

There are several ideas to deal with this issue, as discussed in bug 747574, such as only gather only the B-Frames and discard the rest, but it needs to be implemented and reviewed.

I am interested in solving the issue if you have a little time to assist me technically. I have sent an invitation to you via linkedin to be able to write you personally and waiting for acception. 

Meanwhile I am going to research on some technical concepts related to this issue.

(In reply to Engin FIRAT from comment #25)
> I am interested in solving the issue if you have a little time to assist me
> technically. I have sent an invitation to you via linkedin to be able to
> write you personally and waiting for acception. 

It is better to discuss it openly, since the change implies to modify a base class, which is shared among almost all the video decoders. I would be better to use the gstreamer mailing list:

http://lists.freedesktop.org/mailman/listinfo/gstreamer-devel

Okey I will create a new thread.

This is the link of the new thread. 
http://lists.freedesktop.org/archives/gstreamer-devel/2015-September/054473.html

Created attachment 328420 [details] [review]
vaapidecoder_h264: make sure to confirm if decoder is initialized when starting to decode a frame

See commit message.

In details, during reverse playback, upstream sends bunch of buffers.
base decoder get all those buffers and keep until discont buffer reaches.

eg. | 1 2 3 4 5 6 7 8 9(discont) |
      K       K       

In this case, base decoder parse all these buffers, starts to decode 1 2 3 4
and push decoded buffers to downstream. And then trys to continue to decode 5 6 7 8 9,
At the moment, crash happens.
Because base decoder calls flush method, which does full-reset in vaapi decoder.

This patch looks workaround, but I want to share this problem.

Created attachment 328781 [details] [review]
vaapidecoder: drop non-keyframe in reverse playback and create new vmethod for ensure_decoder

Creating new vmethod to avoid a crash, which is mentioned above, is better than touching each child class.

Review of attachment 328781 [details] [review]:

Hey! 

I just tested it. As we talked, it is not perfect, there are bad spots, but overall it is a great achievement!

A couple comments:

I would like to split this patch in two: one for the vmethod and other for the frame dropping in decoder.

We need to find a way to make the reverse playback terser. Have you checked GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT?? Perhaps we could avoid the deadlock with it.

::: gst/vaapi/gstvaapidecode.c
@@ +537,3 @@
+  if (decode->in_segment.rate < 0.0
+      && !GST_VIDEO_CODEC_FRAME_IS_SYNC_POINT (out_frame)) {
+    GST_DEBUG_OBJECT (decode, "drop frame in reverse playback");

I would use GST_TRACE_OBJECT to avoid a flood of debug messages

@@ +1247,3 @@
+  }
+
+  ret = GST_VIDEO_DECODER_CLASS (parent_class)->sink_event (vdec, event);

This ret variable seems not need. I would code it like

switch (GST_EVENT_TYPE (event)) {
case GST_EVENT_SEGMENT:
  gst_event_copy_segment (event, &decode->in_segment);
  break;
default:
  break;
}

return GST_VIDEO_DECODER_CLASS (parent_class)->sink_event (vdec, event);

Review of attachment 328781 [details] [review]:

::: gst/vaapi/gstvaapidecode.c
@@ +538,3 @@
+      && !GST_VIDEO_CODEC_FRAME_IS_SYNC_POINT (out_frame)) {
+    GST_DEBUG_OBJECT (decode, "drop frame in reverse playback");
+    gst_video_decoder_drop_frame (GST_VIDEO_DECODER (decode), out_frame);

Oh, I forgot, in my opinion we should use here gst_video_decoder_relese_frame, because we don't want a QoS message, just silently discard this frame:

https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-drop-frame

Another alternative is to modify GstVideoDecoder to push the current gathered_list even if it is not a complete GOP, maybe with some NULL buffer or something.

Created attachment 330695 [details] [review]
vaapidecode: drop non-keyframe in reverse playback

(In reply to Víctor Manuel Jáquez Leal from comment #32)
> Review of attachment 328781 [details] [review] [review]:

> Oh, I forgot, in my opinion we should use here
> gst_video_decoder_relese_frame, because we don't want a QoS message, just
> silently discard this frame:
> 
> https://gstreamer.freedesktop.org/data/doc/gstreamer/head/gst-plugins-base-
> libs/html/gst-plugins-base-libs-GstVideoDecoder.html#gst-video-decoder-drop-
> frame

I agree with you.

Created attachment 330696 [details] [review]
vaapidecoder: make sure that decoder is initialized when starting to decode a frame

Splitted.

(In reply to Víctor Manuel Jáquez Leal from comment #33)
> Another alternative is to modify GstVideoDecoder to push the current
> gathered_list even if it is not a complete GOP, maybe with some NULL buffer
> or something.

Thanks for a couple of ideas!
I've been having hard time to improve this feature. :(
I'm going to survey your suggesstions (GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT and the idea above)

> I'm going to survey your suggesstions (GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT
> and the idea above)

About GST_BUFFER_POOL_ACQUIRE_FLAG_DONTWAIT, I played with it yesterday, but I realized that the buffer pool is not linked directly with the VA surfaces: the surfaces are user data of the frames. So that flag won't control anything :(

(In reply to Víctor Manuel Jáquez Leal from comment #33)
> Another alternative is to modify GstVideoDecoder to push the current
> gathered_list even if it is not a complete GOP, maybe with some NULL buffer
> or something.

@Victor,
I tried this idea, but I think the result wouldn't be that different from only key frame decoding.
The thing is that we should push buffers in reverse order, which means that we should decode all frames from key frame.
If we want to show more frames(probably 2-3 frames more), we could do inside vaapi within surface capacity. But I don't think it's real improvement from only key frame decoding.

(In reply to Hyunjun Ko from comment #35)
> Created attachment 330696 [details] [review] [review]
> vaapidecoder: make sure that decoder is initialized when starting to decode
> a frame
> 
> Splitted.

As you mentioned in comment #29, this is a workaround. The problem is that the decoder calls ensure_codec() per frame, adding non-trivial amount of overhead.

So NAK. IMO, we need to fix decoder's flush() vmethod.

Created attachment 331191 [details] [review]
vaapidecode: drop non-keyframe in reverse playback

To avoid surface-exhausted situation during reverse playback,
drop frames except for key frame.

Also, to avoid the corruption of the parser state, flush() vmethod
doesn't destroy the VA decoder when playing in reverse.

https://bugzilla.gnome.org/show_bug.cgi?id=742922

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 331192 [details] [review]
decoder: vc1: flush dpb only if opened

Flush the decode picture buffer, if and only if, the decoder is
started. Otherwise the dpb structure might be NULL.

@Hyunjun

could you test these patches (remember to apply first the patches in bug 768652), please :D ???

(In reply to Víctor Manuel Jáquez Leal from comment #42)

> could you test these patches (remember to apply first the patches in bug
> 768652), please :D ???

I tested with h265,h265,mpeg2,vp8,vc1. All is fine :)

(In reply to Hyunjun Ko from comment #43)
> (In reply to Víctor Manuel Jáquez Leal from comment #42)
> 
> > could you test these patches (remember to apply first the patches in bug
> > 768652), please :D ???
> 
> I tested with h265,h265,mpeg2,vp8,vc1. All is fine :)

Cool! Pushing then!

Attachment 331191 [details] pushed as 19c0c8a - vaapidecode: drop non-keyframe in reverse playback
Attachment 331192 [details] pushed as fcc0862 - decoder: vc1: flush dpb only if opened