Opened 9 years ago

Closed 4 years ago

#6564 closed enhancement (fixed)

[MediaPlayer] show duration in playlist window (easy)

Reported by: diver Owned by: stippi
Priority: normal Milestone: R1
Component: Applications/MediaPlayer Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description


Attachments (5)

mediaplayer.patch (8.9 KB) - added by TwoFx 4 years ago.
No functional change compared to previous verson
mediaplayer.2.patch (12.1 KB) - added by TwoFx 4 years ago.
v4
mediaplayer.3.patch (12.1 KB) - added by TwoFx 4 years ago.
v4.1
0001-MediaPlayer-Show-individual-track-length-in-playlist.patch (8.6 KB) - added by TwoFx 4 years ago.
mediaplayer2.patch (9.9 KB) - added by TwoFx 4 years ago.
Individual duration & visual improvements

Download all attachments as: .zip

Change History (23)

comment:1 Changed 8 years ago by halilpk

Haiku revision: hrev42211 still a bug. There was no duration in playlist window . System: Haiku hrev1-alpha3 on VMware workstation 8 on windows 7 32 bit

comment:2 Changed 7 years ago by harvard

Media Player shows duration in playlist window in Haiku R1/Alpha 4.1

comment:3 Changed 7 years ago by diver

Maybe I'm missing something, but it hasn't been implemented yet.

comment:4 Changed 4 years ago by Barrett

Summary: [MediaPlayer] show duration in playlist window[MediaPlayer] show duration in playlist window (easy)

comment:5 Changed 4 years ago by TwoFx

I had a look at this for Google Code-In. I implemented the functionality by probing all files in the playlist when the playlist window is created and then tracking incremental changes using a listener to avoid having to rescan a potientially large playlist. However, the performance impact at startup and when adding large amounts (50+) of files is still noticeable in my virtual machine, though that was probably to be expected as determining the length of a media file can be a rather expensive operation. I will attach my patch.

comment:6 Changed 4 years ago by TwoFx

Has a Patch: set

comment:7 Changed 4 years ago by pulkomandy

You can make the operation a lot less expensive by first looking if the file has the "be:media_duration" attribute set. This is a standard attribute indicating the duration, which is there exactly to avoid the need to repeatedly scan for the duration of a file.

If the attribute is not set yet, you can use your code to determine it, and you can also set it, so the next time the file is probed, the operation will be faster.

Also, 50 files is not really a very large playlist. If using the attribute doesn't speed this up enough, I would go with a different strategy, either dynamically finding the duration when the items are actually shown in the playlist window, or computing the durations in a background thread and updating the playlist as they are found. This would allow immediately showing the list.

comment:8 Changed 4 years ago by Barrett

The main problem of looking for attributes is that there's not any assurance that it's set. For example you might add a track from a non befs volume. If that attribute is available, this is good but I'll propose another solution in the general case.

One possible solution you can look is to create a temporary BMediaFile before to add the track to the playlist, once a (set of) track(s) have to be processed, you will use BMediaFile::SetTo() to reset that BMediaFile to your specific entry_ref each time. You can then cache this value (by adding an attribute to the class) so that you don't have to build/delete the TrackSupplier(s) which is expensive. Also CreateTrackSupplier() is doing something similar but this way you can optimize that operation.

comment:9 Changed 4 years ago by stippi

No matter how well it is optimized, it should definitely be an asynchronous operation in a separate thread. It cannot happen in the window thread or it will certainly be a problem in a number of situations.

comment:10 Changed 4 years ago by TwoFx

I have made some improvements to the patch. If the playlist is dealing with a file (which, in practice, it does 100% of the time I think, but the interface doesn't require it to be this way), the BMediaFile interface is used directly to get the duration. All querying happens on a dedicated BLooper thread. This solution runs faster than the old one and does not impact startup time or lock up the UI when adding files to a playlist.

This does not yet add a duration attribute as neither Google nor grep have helped me to find out more about this attribute. Once I know which code and format this attribute has, it will be very straightforward to add the to the implementation as PlaylistItem already has complete support for attributes.

Changed 4 years ago by TwoFx

Attachment: mediaplayer.patch added

No functional change compared to previous verson

comment:11 Changed 4 years ago by pulkomandy

PlaylistWindow::_GetInitialDuration() This will generate a BMessage for each item. You could instead put all items in a single message? (you can add the same entry several times to a message, then use a for loop at the receiving side to get them all)

PlaylistWindow::_UpdateDuration(bigtime_t duration)

Instead of using the << operator for concatenating, please use BString.SetToFormat. This will give more control to people localizing it (maybe they will need the duration before the label in some languages).

General comments: there is no clear identification of the "total duration" vs. individual file. I would try to name the methods accordingly to avoid confusion.

About the attribute: you couldn't find anything because I gave you the wrong name. It is named Media:Length. You can see it in the video and audio supertypes (https://github.com/haiku/haiku/blob/master/src/data/mime_db/video.super) and the ML thread where it was defined: http://www.freelists.org/post/haiku-development/Defining-more-standard-attributes,25

Changed 4 years ago by TwoFx

Attachment: mediaplayer.2.patch added

v4

Changed 4 years ago by TwoFx

Attachment: mediaplayer.3.patch added

v4.1

comment:12 Changed 4 years ago by Barrett

Resolution: fixed
Status: newclosed

Patch applied in hrev49985. Thanks!

comment:13 Changed 4 years ago by Coldfirex

Barrett: Is it just me or is that the wrong patch for this issue.

comment:14 Changed 4 years ago by Coldfirex

*Revision

comment:15 in reply to:  13 Changed 4 years ago by TwoFx

Replying to Coldfirex:

Barrett: Is it just me or is that the wrong patch for this issue.

It's correct. hrev49985 applied several unrelated changes, including commits http://cgit.haiku-os.org/haiku/commit/?id=beddc8eefa12b83e06b959605bda94a6a9ec83ed and http://cgit.haiku-os.org/haiku/commit/?id=1c68b67a36bddb3f444094c305c6a5f3868d77ee.

comment:16 Changed 4 years ago by diver

Resolution: fixed
Status: closedreopened

Playlist only shows total playlist length. But with I meant with this ticket is to show duration for every track.

comment:17 Changed 4 years ago by TwoFx

I am sorry for the misunderstanding. I will attach another patch that adds this functionality.

Changed 4 years ago by TwoFx

Attachment: mediaplayer2.patch added

Individual duration & visual improvements

comment:18 Changed 4 years ago by Barrett

Resolution: fixed
Status: reopenedclosed

Applied in hrev49997.

Note: See TracTickets for help on using tickets.