Opened 14 years ago
Closed 9 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: | ||
Platform: | All |
Description
Attachments (5)
Change History (23)
comment:1 by , 13 years ago
comment:4 by , 9 years ago
Summary: | [MediaPlayer] show duration in playlist window → [MediaPlayer] show duration in playlist window (easy) |
---|
comment:5 by , 9 years ago
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 by , 9 years ago
patch: | 0 → 1 |
---|
comment:7 by , 9 years ago
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 by , 9 years ago
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 by , 9 years ago
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 by , 9 years ago
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.
by , 9 years ago
Attachment: | mediaplayer.patch added |
---|
No functional change compared to previous verson
comment:11 by , 9 years ago
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
comment:12 by , 9 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Patch applied in hrev49985. Thanks!
follow-up: 15 comment:13 by , 9 years ago
Barrett: Is it just me or is that the wrong patch for this issue.
comment:15 by , 9 years ago
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 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Playlist only shows total playlist length. But with I meant with this ticket is to show duration for every track.
comment:17 by , 9 years ago
I am sorry for the misunderstanding. I will attach another patch that adds this functionality.
by , 9 years ago
Attachment: | 0001-MediaPlayer-Show-individual-track-length-in-playlist.patch added |
---|
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