Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 4 years ago

Closed 14 months ago

#8631 closed Developer Task (Fixed)

Treat cutlists like lossless transcoding

Reported by: Jim Stichnoth <stichnot@…> Owned by: stichnot
Priority: minor Milestone: 0.26.1
Component: MythTV - Video Playback Version: Unspecified
Severity: medium Keywords:
Cc: Ticket locked: no

Description

This patch makes playback when a cutlist is present behave much more like after lossless transcoding. In particular:

  1. The playback progress indicator reflects the cutlist. For example, it might display "Play 7:43 of 42:44" instead of "Play 9:39 of 60:00".
  1. Seek operations, such as jumping forward or backward or to a particular minute mark, also take the cutlist into account.

I'm a little concerned that this introduces even more conversions back and forth between frames and seconds, but that cleanup is beyond the scope of this patch...

Attachments (9)

seek_honors_cutlist.patch (8.6 KB) - added by Jim Stichnoth <stichnot@…> 4 years ago.
seek_honors_cutlist_v2.patch (22.3 KB) - added by Jim Stichnoth <stichnot@…> 4 years ago.
seek_honors_cutlist_v3.patch (25.2 KB) - added by Jim Stichnoth <stichnot@…> 4 years ago.
seek_honors_cutlist_v4.patch (25.6 KB) - added by Jim Stichnoth <stichnot@…> 3 years ago.
seek_honors_cutlist_v5.patch (27.1 KB) - added by Jim Stichnoth <stichnot@…> 3 years ago.
seek_honors_cutlist_v6.patch (27.0 KB) - added by Jim Stichnoth <stichnot@…> 3 years ago.
seek_honors_cutlist_v8.patch (28.7 KB) - added by Jim Stichnoth <stichnot@…> 3 years ago.
seek_honors_cutlist_v11.patch (27.8 KB) - added by Jim Stichnoth <stichnot@…> 2 years ago.
seek_honors_cutlist_v12.patch (28.2 KB) - added by Jim Stichnoth <stichnot@…> 2 years ago.
Fixed a long-standing bug in DeleteMap::TranslatePositionAbsToRel?().

Download all attachments as: .zip

Change History (24)

Changed 4 years ago by Jim Stichnoth <stichnot@…>

comment:1 Changed 4 years ago by robertm

  • Owner changed from ijr to mdean
  • Status changed from new to assigned

Changed 4 years ago by Jim Stichnoth <stichnot@…>

comment:2 Changed 4 years ago by Jim Stichnoth <stichnot@…>

Added v2 patch, which follows Mike Dean's suggestions to make both the pre- and post-cutlist information available to the themer, and to introduce additional keybindings to allow seeking with and without taking the cutlist into account.

TV Playback keybindings added are ARBSEEKNOCUTLIST, INFONOCUTLIST, JUMPFFWDNOCUTLIST, JUMPRWNDNOCUTLIST, SEEKFFWDNOCUTLIST, and SEEKRWNDNOCUTLIST.

Theme fields added are rawposition, rawdescription, rawplayedtime, rawtotaltime, rawremainingtime, and rawbehindtime. (The last one is probably unnecessary.)

Network Control "PLAY SEEK" commands added are FORWARDNOCUTLIST, BACKWARDNOCUTLIST, and POSITIONNOCUTLIST. (It might be cleaner to add an optional NOCUTLIST argument to the existing FORWARD/BACKWARD/POSITION arguments.)

Note that the patch makes the default behavior to seek and display taking the cutlist into account. This is based on the assumption that if the user went to the effort of creating a cutlist, then she probably wants to see the effect in most parts of the user experience.

Changed 4 years ago by Jim Stichnoth <stichnot@…>

comment:3 Changed 4 years ago by Jim Stichnoth <stichnot@…>

Added v3 patch, which makes additional fields available in the osd_program_editor theme context. "reltimedisplay" and "rellengthdisplay" are the equivalent to "timedisplay" and "lengthdisplay" with the current cutlist applied. "fulltimedisplay" has the pattern "timedisplay (reltimedisplay of rellengthdisplay)", e.g.:

0:16:23.09 (12:35 of 42:34)

The nice thing here is that while editing, you can see a running summary of how much of the recording is left over after the cutlist is applied.

The patch includes a sample usage in default-wide/osd.xml.

comment:4 Changed 3 years ago by tralph

(In [27326]) Several changes to fix the current position and duration of the playback OSD

  • Start using the currently displayed video timecode for the OSD position instead of frame number and total frames which is inaccurate.
  • Use FFmpeg duration for pre-recorded and video playback length. LiveTV is already correct since it uses the actual recording times but in-progress recordings still uses frames written and fps which is obviously wrong for variable framerate and repeat frame material. The problem with in-progress duration should be able to be fixed by using recording times like livetv does. An update to mythplayer and the recorder class will be necessary.
  • Refactor and clean-up some timestamp/timecode handling.

Refs #8631.

Fixes #9109.

comment:5 Changed 3 years ago by markk

I'm going to go 'on record' and state that I don't think this patch should be committed.

Firstly, I personally find it counterintuitive to display a different program duration (or position) from the actual, known value. The potential for confusion is huge.

Secondly, any confusion can only be mitigated by themers competently using the extra information available. Given that this is only likely to be a niche feature, I can see few themes actually doing that.

Finally, it adds a level of complexity to the playback code that we could really do without. We are trying to make this code simpler and more understandable and the 'cost' here does not outweigh the 'benefit' in my opinion.

comment:6 Changed 3 years ago by Jim Stichnoth <stichnot@…>

See my response to Mark's comment on the dev list.

In case anyone else is using this patch, I'll continue to update the patch here if the ticket gets closed.

Changed 3 years ago by Jim Stichnoth <stichnot@…>

Changed 3 years ago by Jim Stichnoth <stichnot@…>

Changed 3 years ago by Jim Stichnoth <stichnot@…>

Changed 3 years ago by Jim Stichnoth <stichnot@…>

Changed 2 years ago by Jim Stichnoth <stichnot@…>

Changed 2 years ago by Jim Stichnoth <stichnot@…>

Fixed a long-standing bug in DeleteMap::TranslatePositionAbsToRel?().

comment:7 Changed 2 years ago by wagnerrp

  • Owner changed from mdean to stichnot
  • Type changed from enhancement to Developer Task

comment:8 Changed 2 years ago by stichnot

  • Component changed from MythTV - General to MythTV - Video Playback
  • Milestone changed from unknown to 0.26

comment:9 Changed 2 years ago by Jim Stichnoth <jstichnoth@…>

In 769059b1f85af7963574cff0d14c74e387ab2422/mythtv:

Add functions for converting between pre- and post-cutlist frame numbers.

Refs #8631. These functions are not actually used anywhere yet.

comment:10 Changed 2 years ago by Jim Stichnoth <jstichnoth@…>

In 990e579f724bb2d1af87be98028ad664a5795bc0/mythtv:

Add new OSD theme strings for cutlist-adjusted times and durations.

The following strings are added, related to the corresponding string
without the "rel" prefix: relsecondsdisplayed, reltotalseconds,
relposition, reldescription, relplayedtime, reltotaltime,
relremainingtime, relbehindtime.

In addition, a progress bar called "relposition" is added, which is
the cutlist-adjusted version of the "position" progress bar.

Note: MythPlayer::calcSliderPos() isn't indented properly so that the
diffs are more apparent. That will be fixed.

Refs #8631.

comment:11 Changed 2 years ago by Jim Stichnoth <jstichnoth@…>

In f0b202909eaeb963a6c3648f9eb45ab3e95ba159/mythtv:

Take the cutlist into account when seeking during playback.

Cut regions are honored when calculating exactly where to jump.

Temporarily, the "INFOWITHCUTLIST" key binding is added for the
Arbitrary Seek functionality. By default, "INFO" will jump to an
absolute (non cutlist adjusted) point in the recording, whereas
"INFOWITHCUTLIST" with do a cutlist-adjusted jump. This can be
removed when we work out how to allow the playback progress bar to
convey cutlist information.

Refs #8631.

comment:12 Changed 2 years ago by Jim Stichnoth <jstichnoth@…>

In ed316911fd32bf31def19705adc1057f6b0dc9d1/mythtv:

Revert "Fix seeking backwards into a cut region."

This reverts commit 89c34ef6b2e0e8688474ef546b799d54d8ec6242.

This is no longer needed after f0b202909eaeb963a6c3648f9eb45ab3e95ba159.

Refs #8631. Refs #9213.

comment:13 Changed 2 years ago by Jim Stichnoth <jstichnoth@…>

In 4379cfab38e16f1a84d7cbf440489686e5ff7eb9/mythtv:

Refs #8631. Set OSD relposition property for timestretch, volume, etc.

comment:14 Changed 21 months ago by stichnot

  • Milestone changed from 0.26 to unknown

The only remaining piece here is to remove the temporary INFOWITHCUTLIST action, which can happen after we implement progressbar visualization of cutlist (and other types of marks) visualization.

comment:15 Changed 14 months ago by stichnot

  • Milestone changed from unknown to 0.26.1
  • Resolution set to Fixed
  • Status changed from assigned to closed

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.