Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

setvideopos doesn't work any more #313

Closed
gkreidl opened this issue Feb 13, 2015 · 17 comments

Comments

@gkreidl
Copy link

@gkreidl gkreidl commented Feb 13, 2015

In latest release from http://omxplayer.sconde.net/ the dbus command setvideopos doesn't work any more. The video gets lost.
I'm using that a lot in omxplayerGUI, when a window is moved or resized or the display format is changed.
It works with the default Raspbian version.

@ktws

This comment has been minimized.

Copy link

@ktws ktws commented Feb 16, 2015

@gkreidl I submitted a bugfix for this 17 days ago, #307, but unfortunately it's not yet been accepted.

Frustratingly, omxplayer has been upgraded in the repositories today without this bug fix. This means that any software using 'setvideopos' and/or 'unhidevideo' d-bus commands will no longer work with the default Raspbian version.

@popcornmix Any chance of getting pull request #307 accepted and the repository version upgraded accordingly?

@subogero

This comment has been minimized.

Copy link

@subogero subogero commented Feb 16, 2015

Hi. I don't know about the fix, but the latest omxplayer builds from skgsegio's build bot are available in my Deb repo. So once once the fix is accepted, you can use that. More info at the bottom of the page at http://subogero.github.io/remotepi

@gkreidl

This comment has been minimized.

Copy link
Author

@gkreidl gkreidl commented Feb 16, 2015

I strongly support ktws's suggestion to add the bug fix and get it into the repository as fast as possible.

@popcornmix

This comment has been minimized.

Copy link
Owner

@popcornmix popcornmix commented Feb 16, 2015

#307 is now merged.

@gkreidl

This comment has been minimized.

Copy link
Author

@gkreidl gkreidl commented Feb 16, 2015

Thanks! :-)
How long might it take to get the new version into the repository?

@42loop

This comment has been minimized.

Copy link

@42loop 42loop commented Mar 11, 2015

hm, this still does not work. with #307 'setvideopos' doesnt stop the video anymore, but is of no effect (besides a hiccup in the video).
'unhidevideo' does not work
[still trying to figure out how its supposed to work, maybe we just need a simple setvideorect...]

@gkreidl

This comment has been minimized.

Copy link
Author

@gkreidl gkreidl commented Mar 11, 2015

I can assure you that both commands do work with the latest release. I'm using them all the time.

@ktws

This comment has been minimized.

Copy link

@ktws ktws commented Mar 11, 2015

#307 works but you need to download and install omxplayer_0.3.6git201502175337be8_armhf.deb from http://omxplayer.sconde.net/

sudo dpkg -i omxplayer_0.3.6~git20150217~5337be8_armhf.deb

I didn't really document the d-bus commands other than in my initial commit, #149, and examples within dbuscontrol.sh. Hopefully the following descriptions will help you understand what they do.

  • hidevideo hides the video while retaining sound
  • unhidevideo video restores the video to its original position
  • setvideopos lets you change the video on-screen position/size
@42loop

This comment has been minimized.

Copy link

@42loop 42loop commented Mar 11, 2015

ok, the build from sconde.net does it, however the current git source which i'm compiling myself does not work. strange enough, 'hidevideo' and all other dbus commands work fine

@popcornmix

This comment has been minimized.

Copy link
Owner

@popcornmix popcornmix commented Mar 11, 2015

Can you try building from 5337be8
which should match the sconde build. Assuming that works, you can identify the specific build that broke it (there's only been 6 commits since then).

@42loop

This comment has been minimized.

Copy link

@42loop 42loop commented Mar 11, 2015

the sconde build is of version 5337be8 - that one works
until version c5ab808 it also works
from version 14237dd it does not work anymore, so it is #322 that breaks it. if i undo the changes from #322 in omxplayer.cpp, e.g. replacing the reset call by the old calls to close, open, it works. something in the reset function must be odd...

@popcornmix

This comment has been minimized.

Copy link
Owner

@popcornmix popcornmix commented Mar 11, 2015

ping @tdicola

@tdicola

This comment has been minimized.

Copy link

@tdicola tdicola commented Mar 11, 2015

Oh weird, let me take a look and repro tonight. Is it only when changing position through dbus or also from the keyboard that it fails (I suspect it would fail with keyboard too, but just want to check)?

@ktws

This comment has been minimized.

Copy link

@ktws ktws commented Mar 11, 2015

I'll have a proper look later tonight but it looks like the "m_has_video" flag is the problem. For example:

KeyConfig::ACTION_HIDE_VIDEO (1416-1424) sets "m_has_video" to false and closes "m_player_video" in order to hide the video layer.

I chose to code it this way as there's no "stutter" interruption in sound when removing the video layer. It makes for a better user experience in the media player wrapper i'm writing.

If I understand correctly, #322 introduces line 1497 which doesn't re-open "m_player_video" in order to optimize video restart.

@ktws

This comment has been minimized.

Copy link

@ktws ktws commented Mar 12, 2015

@tdicola using the old code when "m_new_win_pos" is true corrects the problem:

      if (!m_new_win_pos)
      {
        if (m_has_video && !m_player_video.Reset())
          goto do_exit;
      }
      else
      {
        if (m_has_video && !m_player_video.Open(m_hints_video, m_av_clock, DestRect, m_Deinterlace ? VS_DEINTERLACEMODE_FORCE:m_NoDeinterlace ? VS_DEINTERLACEMODE_OFF:VS_DEINTERLACEMODE_AUTO,
                                           m_anaglyph, m_hdmi_clock_sync, m_thread_player, m_display_aspect, m_display, m_layer, video_queue_size, video_fifo_size))
          goto do_exit;
      }

@tdicola

This comment has been minimized.

Copy link

@tdicola tdicola commented Mar 12, 2015

Ah, yeah that looks like a good way to fix it so the player is opened again after hiding and unhiding the video. Just about have my fork setup and compiling again so I'll give it a quick test and send a pull with the change soon.

@tdicola

This comment has been minimized.

Copy link

@tdicola tdicola commented Mar 12, 2015

Yeah that fix seems to work well, thanks! I can use the hide, unhide, and setvideopos commands from dbus and they seem to work as expected now. Sent a pull with the fix here: #326

tdicola added a commit to adafruit/omxplayer that referenced this issue Mar 12, 2015
popcornmix added a commit that referenced this issue Mar 13, 2015
Fix #313 by catching when unhide occurs and explicitly re-opening video player
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.