Bug 419561

Created attachment 283941 [details]
Patch: don't add ASX entries with a duration of 00:00:00 to the playlist

Created attachment 283951 [details]
The same patch for 2.21.3

No, this should be done in the front-ends, not the parser itself.
The browser plugin already has such code, we'd need that code in the main Totem
itself (see src/totem-playlist.c).

(In reply to comment #3)
> No, this should be done in the front-ends, not the parser itself.

Hmm, why? Then the wheel needs to get reinvented each time, anyways if you
really want it that way I'll happily adapt my patch to make it so.

> The browser plugin already has such code, we'd need that code in the main Totem
> itself (see src/totem-playlist.c).

Well it doesn't seem to have such code for this case as opening for example:
http://www.rtl.nl/(vm=/service/gemist/home/,id_by_component)/system/video/html/components/reality/rtlprojectcatwalk/miMedia/2007/week50/ma_projectcatwalk_10dec.avi_plain.xml

Where the plugin gets loaded, doesn't work (not even when all the ugly / bad
ffmpeg plugins needed are present), and with my plparser patch it does, or am I
misunderstanding and does src/totem-playlist.c get used by both the plugin and
the player?

Also src/totem-playlist.c is not a small file I've taken a quick look but I
couldn't find an obvious place to add a check for 00:00:00 duration files, which
function should I look at?

$ ./test-parser file:///tmp/totem/browser-plugin/1537.wvx 

###################### parsing ################

added URI 'http://www.rtl.nl/system/transparant-pixel.gif'
        duration = '00:00:00'
        author = 'RTL Nederland'
added URI
'mmsh://idols-av.rtl.nl/web/components/shows/idols4/video/2007/week49/vr_nathalie_achteraf.avi.MiMedia_WM_1500K_V9_idols-av_film_an_t4b4l2r2_716x402.wmv?MSWMExt=.asf'

And in browser-plugin/totem-plugin-viewer.c:
       /* Skip short advert streams */
       duration = totem_pl_parser_parse_duration (g_hash_table_lookup (metadata,
"duration"), FALSE);
       if (duration == 0)
              return;

I also noticed that we already ignore 0-length items in playlists in
totem_playlist_entry_parsed() in trunk.

So it's already fixed in rawhide (despite what you put in the version field for
the report). Feel free to add your work-around to F-8 though.

(In reply to comment #5)
> So it's already fixed in rawhide (despite what you put in the version field for
> the report).

Sorry about that.

> Feel free to add your work-around to F-8 though.
You mean the attached patch for plparser you didn't want upstream? I guess it
would be better then to just add these 2 patches from upstream:
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866

(With some adjustments to apply and compile properly)

Shall I add those 2 to F-8, commit build and push to updates-testing?

Created attachment 285851 [details]
Proposed F-8 patch

Here is a proposed patch for F-8 based on the upstream fix for this.

(In reply to comment #6)
> (In reply to comment #5)
> > So it's already fixed in rawhide (despite what you put in the version field for
> > the report).
> 
> Sorry about that.

No worries, happens to me as well.

> > Feel free to add your work-around to F-8 though.
> You mean the attached patch for plparser you didn't want upstream?

> I guess it
> would be better then to just add these 2 patches from upstream:
>
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866
>
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866
> 
> (With some adjustments to apply and compile properly)
> 
> Shall I add those 2 to F-8, commit build and push to updates-testing?

Yeah, looks fine actually. Thanks!

I committed that patch to the 2.20 branch, so we can remove it next time I make
a release.

Patch committed, build and push to updates-testing required.

totem-2.20.1-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update totem'

totem-2.20.1-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update totem'

totem-2.20.1-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update totem'

totem-2.20.1-2.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.