Rockbox

Tasklist

FS#7487 - mpegplayer - video start time seek with resume

Attached to Project: Rockbox
Opened by Brian Morey (Morey) - Thursday, 26 July 2007, 18:13 GMT
Last edited by Robert Kukla (roolku) - Tuesday, 09 October 2007, 21:39 GMT
Task Type Patches
Category Video
Status Closed
Assigned To No-one
Operating System All players
Severity Low
Priority Normal
Reported Version Daily build (which?)
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 14
Private No

Details

This patch adds the following functionality to the mpegplayer plugin:

Start up menu for control of movie playback:

The startup menu allows for control where the video files are started. The user can select to play the movie from the beginning, resume based on a bookmark, or select a time in minutes to begin. A slider function has been added to aid in time selection. Additionally, a thumbnail is actively drawn that follows the slider position to show the user visually the position in the movie.

Resume feature:

The resume feature incorporates a circular buffer of 3 movie titles and resume times stored inside the mpegplayer.cfg file. When selecting a movie title, the .cfg file is checked for a resume time on that title, then a startup menu will allow you to play the movie from the beginning or resume (time of 0 if not found) based on the bookmark. The resume bookmark is updated with the last movie start time or the last exit time (not movie ending), whichever occurs last. It the title is not in the .cfg file, a new entry is made in place of the oldest resume entry.

Set start time:

The startup menu allows the user to select a start time to begin play. By using a slider widget to select a number of minutes into the file, a thumbnail of the movie frame at that time will be displayed. Upon selecting the thumbnail of choice the movie will start playback from that time.

NOTES:

This patch has been tested on the Sandisk Sansa e200 series device, the ipod simulator, and the gigabeat simulator.

Please post results of other platforms.

This patch has been written by:
John S. Gwynne
Brian J. Morey
This task depends upon

Closed by  Robert Kukla (roolku)
Tuesday, 09 October 2007, 21:39 GMT
Reason for closing:  Accepted
Comment by Charles Philip Chan (cpchan) - Thursday, 26 July 2007, 23:44 GMT
Just tried out your patch. The resume feature seem to be working more smoothly than the mpegplayer resume patch. With regards to the seeking feature, it would be nicer to be able to seek while playing the movie itself instead of going out to the menu and the resolution of 1 min. seem a bit high. Also, I am not too fond of the arbitrary limit in the number of bookmarks (I know I can increase it in the source).

Charles
Comment by Johnathon Mihalop (Soul-Slayer) - Friday, 27 July 2007, 02:27 GMT
Starts up with the message:

missing packet start code prefix : 00 at 5000

Then works as expected (I think :P)

Gigabeat F20 latest SVN build.

Also, there's no way of leaving the start menu other than playing the video. I'd like it so that if I accidentally choose the wrong video, I can just press a button and return to the file tree, instead of HAVING to play the file and exiting from there. Otherwise great work!
Comment by Matthew Schneider (mschneider) - Friday, 27 July 2007, 15:31 GMT
same message on sansa e260, other than that everything works great. Awesome work!

Also, same request as above. but what would be really useful would be to integrate the startup menu with the mpeg player menu (or even just the seek function).
Comment by Brian Morey (Morey) - Friday, 27 July 2007, 16:37 GMT
We've identified the bug giving this error message and should have a patch out in the next day or two.
Comment by Jacob Gardner (cmptrgy412) - Sunday, 29 July 2007, 23:21 GMT
This is a little bit strange. Probably the mpg file though. I have a 43 minute mpeg file and when I use the seek feature. The seek bar shows 0 - 77 or so. The first 0-43 is really just the first couple minutes and 44 - 77 is the actual timeline. 44 = 2 min. 60 = about 25 min or so. 77 = 43 min.
I've only tried it on one file. It works fine, but that's just a bit annoying.
Sansa e250.
Revision: 14041
Comment by Johnathon Mihalop (Soul-Slayer) - Sunday, 29 July 2007, 23:26 GMT
Same sort of thing, I made one movie file from joining multiple small movie files, and it only seeks as far as the length of the first one (5 mins as opposed to over an hour). If I try and start from anywhere other than 0 mins, it enters an infinite loop of "missing packet start code prefix : *Forgotten the numbers*" and requires a reset. I think this is more a limitation of mpegplayer though, as I believe it reads the timestamps and it seems it still uses the original files timestamps.
Comment by Donald Johnston (donaldj) - Tuesday, 31 July 2007, 14:23 GMT
Works really well. I have the same problem as Soul-Slayer with the start-up message. It would be nice if the 'seek menu' was accessible from the main mpegplayer menu.

Thanks!

-Donald J
iPod Nano
Comment by Andrew Gard (Veralis) - Wednesday, 01 August 2007, 05:55 GMT
I am unable to use mpegplayer with this patch as every video that use to work returns with "plugin returned error" message.
I am using the Gigabeat F40 and I thought I should let you know of these problems.
Comment by Brian Morey (Morey) - Friday, 03 August 2007, 18:23 GMT
This version of the patch should correct the startup errors several of you have encountered. It does not address some of the above issues pertaining to the integrity of the stream. While we expect the player to be able to play broken streams (to some degree:), it is not reasonable to search broken PTS times.

Features added in this patch:
() quit option from the startup menu
Comment by Andrew Gard (Veralis) - Saturday, 04 August 2007, 00:57 GMT
I'm proud to announce that it starts now, but sadly I cant watch video, hear the video sound, and I cant set the resume time.
Comment by Gerritt Gonzales (GRaTT) - Sunday, 05 August 2007, 16:25 GMT
This works great on the sansa. No trouble at all.
All the videos that work without the patch work with it.
GRaTT
Comment by Brian Morey (Morey) - Thursday, 09 August 2007, 20:10 GMT
This version of the patch should now be flexable for more video resolutions. Set video start time now clears the display and centers the thumbnail for better compatibility on different players.

Veralis, please post whether this fixes your issues using the Gigabeat.
Comment by Brian Morey (Morey) - Friday, 10 August 2007, 12:37 GMT
This version of the patch fixes the resume time so that it handles the case for "Quit mpegplayer" from within the movie playback menu. This was an oversight that wasn't calling the store_resume_info function on exit.
Comment by Andrew Gard (Veralis) - Friday, 10 August 2007, 23:46 GMT
I used the patch, and it says "Loading..." and then it goes to a blank black screen, where I can't exit, turn it off, or anything.
Comment by Brian Morey (Morey) - Saturday, 11 August 2007, 02:42 GMT
All I have access to is the simulator, which works with elephants dream. Could you test the elephants dream video on the device to verify that that works.
http://www.rockbox.org/twiki/bin/view/Main/PluginMpegplayer#Elephants_Dream
Comment by Andrew Gard (Veralis) - Saturday, 11 August 2007, 06:54 GMT
I will test it out when I get back to my house as I don't have the software to make that build again right now.
Comment by Andrew Gard (Veralis) - Monday, 13 August 2007, 20:46 GMT
Ok, Elephants Dream works, sorry if I sent you around in circles. I should've tried it but never really thought about it.
Now i just need find a way to encode my videos to work with it now. I have been using Xilisoft Video Converter 3 to encode my old videos that don't work with this patch. So if you can look into making it compatible I would appreciate it.

Thank you for all your hard work on this patch!
Comment by Andrew Gard (Veralis) - Monday, 13 August 2007, 20:51 GMT
Also, here are the details for the encode, I don't have much info but this is all I got.
Elepahnts Dream: Video: MPEG2 Video 320x240 24.00fps 7500Kbps [Video]
Audio: MPEG Audio Layer 3 44100Hz stereo 128Kbps [Audio]

My Videos: Video: MPEG2 Video 320x240 25.00fps 104857Kbps [Video]
Audio: MPEG Audio 44100Hz stereo 192Kbps [Audio]
Comment by Nick Haffen (crackmonkey421) - Monday, 13 August 2007, 21:32 GMT
I've been having the same problem starting my files. The menu loads fine and everything is OK as long as I don't start from the beginning or 0 seconds. My mpeg files work fine with the patchless build. I did not patch this myself, so I'm not completely sure what version of the patch was used, but I tried the unofficial M-Build and cpchan's build (both of which contain your patch) with the same results. I'll figure out how to patch and compile my own a little later (I'm a noob), and I'll post my results. Here are my results so far:

Elephant's Dream works perfectly (aside from Sansa being too slow for full 24 fps)

Files encoded with mencoder (oct 2006's and yesterday's build of mencoder had same results) using the following options work fine until your patch is installed:
mencoder input.avi -of mpeg -oac lavc -lavcopts acodec=mp2:abitrate=128 -af resample=44100:0:0 -ovc lavc -lavcopts vcodec=mpeg2video:vbitrate=250 -vf scale,harddup -ofps 12 -zoom -xy 220 -o output.mpg

As I said before, the only problem with these files is that they won't start from the beginning or 0 seconds. Let me know if you'd rather me just send you an example of a mpeg encoded like this.

Thanks for all the effort... Great work so far!
Comment by Nick Haffen (crackmonkey421) - Tuesday, 14 August 2007, 01:34 GMT
OK...I wanted to see if I still have problems with the latest version of the patch so I just compiled a build with nothing except the latest version of your patch. Same results.. The videos I've encoded with mencoder work fine without the patch, but with it my sansa crashes unless I start somewhere in the middle of the video. I would think it should be able to play the same videos with or without the patch just fine, but it doesn't. And, seeing as Andrew has the same problem on his Gigabeat, it's probably a global problem. I'll take a look at the code some tomorrow, but I'm not sure what good I can do.
Comment by Nick Haffen (crackmonkey421) - Tuesday, 14 August 2007, 06:56 GMT
Mencoder is the only encoder I've tried that doesn't work with this patch. WinFF is easy to use and works fine. To me this patch is worth re-encoding everything for it. http://www.rockbox.org/twiki/bin/view/Main/PluginMpegplayer
Comment by Brian Morey (Morey) - Tuesday, 14 August 2007, 13:15 GMT
Thanks to Nick we now have one of the videos that do not start from 0. We will take a look at this issue.

As far as encoding goes, I have the Sansa player and here is the way I am encoding my videos using mencoder:

mencoder input.vob -of mpeg -oac lavc -lavcopts acodec=mp2:abitrate=192 -af resample=44100:0:0 -ovc lavc -lavcopts vcodec=mpeg2video:vbitrate=469 -vf scale=224:176,harddup,eq2=1.0:2:.4:1.0:1.0:1.0:1.0:1.0 -ofps 15 -o output.mpg

(where input.vob can be any input video)
Comment by Brian Morey (Morey) - Tuesday, 14 August 2007, 16:50 GMT
Video start from time 0 on the above mentioned files should be fixed in this release.
Comment by Nick Haffen (crackmonkey421) - Wednesday, 15 August 2007, 00:42 GMT
excellent! Thanks a lot! I can't find any other problems, but I'll let you know if I do.. I hope this patch gets added soon--its a must have.
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 01:21 GMT
I am getting the following error on my gigabeat build.

missing packet start code prefix: 00 at F000

Is there something I am missing.
Current setup:
Latest Build r14343M
AA, BMP-Resize, & this patch
Gigabeat F40
Mediacoder for encoding videos below are my video settings
Bitrate: 500kbps
Codec: MPEG2
Frame Rate: 30fps
MP3: 128kbps
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 01:23 GMT
Sorry I forgot to add it will play the video but I can't resume or start from a certain point
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 01:30 GMT
I figured out how to start from a certain point. But I have a problem/question, If I play a video and then exit the video it doesn't save my position I have to start from the beginning or try to find my position with the seek feature, is that they way it is supposed to function?
Comment by Johnathon Mihalop (Soul-Slayer) - Wednesday, 15 August 2007, 01:33 GMT
You are using the latest version of the patch right? These all sound like issues that have already been fixed...
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 10:26 GMT
yep I am using patch_onsvn2007-08-14_r1
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 11:53 GMT
I don't get the error playing elephants dream. When get back home today I am going to try encoding my video with VLC instead of Mediacoder to see if that is the problem.
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 13:44 GMT
With further testing it does save my position, but I still get the error when starting the video.
Comment by Jacob Brooks (jac0b) - Wednesday, 15 August 2007, 14:41 GMT
I just used VLC to encode my video and I didn't get the error I got from encoding the video with Mediacoder.
Everything now works perfectly with this plugin on my gigabeat build.
Comment by Andrew Gard (Veralis) - Friday, 17 August 2007, 21:17 GMT
Brian Morey, if you can, try to look into your code and figure out why the formats are getting so limited.
I would do it myself, but I am not big on any type of coding.

Also, I'm going to try a different encoder but mediacoder set with:

Video: 320x240 (I've tried from 600kbps to 2000kbps)
Format: MPEG
Container: MPEG
Audio: LAME MP3 44.1khz Stereo CBR 128KBPS

And everytime I start playback I get the beginning skipping, slowing down almost to a stop, and then slowly going up to speed and playing normally.
Comment by Andrew Gard (Veralis) - Friday, 17 August 2007, 21:18 GMT
Brian Morey, if you can, try to look into your code and figure out why the formats are getting so limited.
I would do it myself, but I am not big on any type of coding.

Also, I'm going to try a different encoder but mediacoder set with:

Video: 320x240 (I've tried from 600kbps to 2000kbps)
Format: MPEG
Container: MPEG
Audio: LAME MP3 44.1khz Stereo CBR 128KBPS

And everytime I start playback I get the beginning skipping, slowing down almost to a stop, and then slowly going up to speed and playing normally.
I also encounter an error before the video loads of "Missing Packet Start Code Prefix: 00 at FF00"
Comment by Brian Morey (Morey) - Monday, 20 August 2007, 02:52 GMT
I've used mediacoder to encode several different movies now and all of them play without error on the gigabeat simulator. I'm unable to create a video stream that shows any problems. Since I don't have access to the actual device please verify that you see these problems in the simulator as well.
Comment by Andrew Gard (Veralis) - Monday, 20 August 2007, 05:09 GMT
Can you tell me the settings you are using exactly?
Comment by Brian Morey (Morey) - Monday, 20 August 2007, 13:47 GMT
MediaCoder: 0.6.0 build 3855

Video
Mode: Bitrate-based 800kbps
Format: MPEG1
Container: MPEG1

Audio
Encoder: LAME MP3
Resample: 44100 Hz
Channel: Stereo
RateMode: CBR
Bitrate/Quality: 128 kbps

Everything else is set to auto.
Comment by Brian Morey (Morey) - Thursday, 23 August 2007, 00:43 GMT
This version of the patch changes the way resume is stored by adding more capability to the config file routines. Resume entries now only take up one line and can be updated without rewriting the entire config file. This allows for virtually unlimited mpeg resume entries. From the video playback menu an option has been added to display the number of resume entries in the config file and allow the user to clear them if they wish.

The following features have been added:
() Removed the software limits on number of files to resume
() Added resume clear option from video playback menu
Comment by Brian Morey (Morey) - Thursday, 23 August 2007, 19:21 GMT
This version of the patch adds better resolution to the start time seek by using half minutes. All resume and seek times are 30 second intervals instead of 60. The display still shows time in minutes with the addition of .5 to denote half minutes.

The following features have been added:
() Seek time is now done in half minutes
Comment by Andrew Gard (Veralis) - Friday, 24 August 2007, 04:32 GMT
Ok, your last release is the greatest thing you have ever done.

I can play all my old videos without re-encoding them. So great work!

Everything for the Gigabeat-F40 is working perfectly!

THANK YOU!
Comment by Steve Bavin (pondlife) - Friday, 24 August 2007, 08:54 GMT
This seems like useful stuff, but to increase the chance of it getting committed you would be better off having multiple small patches, each of which addresses a particular issue or feature.
Comment by Brian Morey (Morey) - Wednesday, 29 August 2007, 14:36 GMT
This version of the patch fixes a lockout issue when using the diagnostic limitFPS setting. When setting limitFPS to no, then restarting the mpegplayer, the player no longer becomes unresponsive to user commands.
Comment by Nick Waterton (Nick_W) - Friday, 31 August 2007, 14:32 GMT
This patch works well, but I did find a couple of things that could do with fixing:

The lines (two of them) in mpeg_settings.c
draw_slider(preview_ypos+preview_height+20, play_time, resume_time);
references the display of the slider to the preview position. This dosen't work on players with very small screens (like my iPod nano). The screen height is 132, with a preview size of 90 this puts the top of the slider at 131 ypos (off the bottom of the screen). I have changed this to:
draw_slider(LCD_HEIGHT-29, play_time, resume_time);
Which puts the slider at the same position relative to the bottom screen all the time.

The second thing that I found is that if the video is encoded using B frames (which I do, as it can improve the fps) then the frame displayed in the preview is usually a B frame, and is mostly garbage.

I changed (in mpeglayer.c)
if (video_thumb_print)
video_str.status = STREAM_STOPPED;
to:
if (video_thumb_print && ((int)(info->current_picture->flags & PIC_MASK_CODING_TYPE) == PIC_FLAG_CODING_TYPE_I))
video_str.status = STREAM_STOPPED;
so that the prieview display will only display an I frame. This does result in a slight (about .5 sec) delay while the next I frame is found, so I'm not sure if the delay for I frames vs the value in using B frames is really justified. It might be better not to use B frames at all, however if you don't use B frames, there is no delay, and everything works as previously - so it may be worth adding just from that point of view. One problem with this is that the play_time stored is the B frame (not the next I frame), so starting from there results in about 0.5 sec's of pixilation. I haven't figured out how to update play_time to store the I frame time yet, but it's not a big deal.

The seek time being 30 seconds is a great improvement, but I think that what you should really do is base the seek time on the playing time of the video, so if the video is say 5 mins long (music video) the seek time is shorter, say 15 seconds, and if the playing time of the video is longer (movie or something) the seek time is 30 seconds. I think I know how to do this, and I'll have a go at implementing it, but it's just a suggestion.

Comment by Brian Morey (Morey) - Saturday, 01 September 2007, 01:44 GMT
This patch fixes the issue (Nick_W) brought up with encoding using B frames. This should now work with preview and start of play (I, B, and P frames).

I'm currently doing tests with the iPod nano simulator to find a solution for preview on the small screen.
Comment by Nick Waterton (Nick_W) - Saturday, 01 September 2007, 18:53 GMT
Brian,

Tried your latest patch, and it works great (much better than my kludge). Thanks for looking at this so quickly. What do you think about the variable seek time idea?
Comment by Brian Morey (Morey) - Sunday, 02 September 2007, 16:53 GMT
The variable seek time is an interesting idea, I'll have to put some more thought into it. The idea of a variable seek time would probably cause mixed reactions. It's very hard to find a solution that fits everyone. I guess I'm inclined to keep the seek time constant for now, rather then selecting some variation based on guessing what people would want.
Comment by chris (oliphant) - Tuesday, 11 September 2007, 23:40 GMT
Hey guys

this patch works great thanks for all the effort that has gone in. I just thought of a minor addition that might be useful: if the fwd and rwd buttons were mapped to the patch this would offer some form of fwd and rwd to the mpegplayer, probably already considered.....

Thanks again, Chris
Comment by Brian Morey (Morey) - Saturday, 22 September 2007, 02:03 GMT
This version of the patch improves video load times. The video will now start on B-frames instead of searching for the first full I-frame. This improvement should load a video file in about half the time of the previous patch.
Comment by Jacob Brooks (jac0b) - Saturday, 22 September 2007, 19:56 GMT
Get a big error with the new patch. No other patches are used just this one.

scramble.o: In function `main':
/rockbox/tools/scramble.c:264: undefined reference to `_gigabeat_s_code'
gigabeats.o: In function `openinfile':
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:25: undefined reference to `fopen'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:28: undefined reference to `stderr'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:28: undefined reference to `fprintf'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:29: undefined reference to `perror'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:30: undefined reference to `exit'
gigabeats.o: In function `openoutfile':
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:37: undefined reference to `fopen'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:40: undefined reference to `stderr'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:40: undefined reference to `fprintf'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:41: undefined reference to `perror'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:42: undefined reference to `exit'
gigabeats.o: In function `gigabeat_s_code':
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:65: undefined reference to `fseek'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:66: undefined reference to `ftell'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:67: undefined reference to `fseek'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:68: undefined reference to `malloc'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:70: undefined reference to `stderr'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:70: undefined reference to `fwrite'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:73: undefined reference to `fread'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:84: undefined reference to `fwrite'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:85: undefined reference to `fwrite'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:95: undefined reference to `fwrite'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:103: undefined reference to `fwrite'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:104: undefined reference to `fwrite'
gigabeats.o:/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:111: more undefined references to `fwrite' follow
gigabeats.o: In function `gigabeat_s_code':
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:166: undefined reference to `stderr'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:166: undefined reference to `fwrite'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:168: undefined reference to `fclose'
/home/bmorey/Projects/rockbox/rockbox_svn_2007-09-21/tools/gigabeats.c:169: undefined reference to `fclose'
collect2: ld returned 1 exit status
make[1]: *** [scramble] Error 1
make: *** [tools] Error 2
Comment by Jacob Brooks (jac0b) - Saturday, 22 September 2007, 20:09 GMT
Sorry should have put in a txt file.
Comment by Brian Morey (Morey) - Sunday, 23 September 2007, 03:55 GMT
These errors are not due to any patch. You have your compiler environment setup wrong.
Comment by Brian Morey (Morey) - Sunday, 23 September 2007, 04:27 GMT
I'm sorry I responded too soon, it turns out that some how my last patch file got corrupt. Try this one.
Comment by Brian Morey (Morey) - Tuesday, 25 September 2007, 14:44 GMT
Re-sync with SVN.
Comment by Nick Waterton (Nick_W) - Tuesday, 25 September 2007, 15:55 GMT
Brian,

Isn't the MPEG_SCROLL_DOWN and MPEG_SCROLL_UP the wrong way round in mpeg_settings.c? I see that MPEG_SCROLL_UP and MPEG_LEFT are the same action (in the case statement) and vice-versa in the MPEG_RIGHT case statement. On my iPod this means that you have to scroll down to fast forward through the video (had me very confused at first!).

I have fixed this in my copy, and now it works the way (I think) that it should. Previous versions didn't do this (they were the other way round) is this just a simple coding error? or to do with other targets (in which case the iPod button mapping should be changed).
Comment by Nick Waterton (Nick_W) - Tuesday, 25 September 2007, 15:57 GMT
I also get a compiler warning "function declaration isn't a prototype" in mpeg_settings.c I'm just not sure which function it's talking about. Maybe I'll look for it later.
Comment by Charles Philip Chan (cpchan) - Wednesday, 26 September 2007, 06:32 GMT
I haven't updated my builds in about 2 weeks and now I am having problems with the patch and the changes in mpegplayer. The patch applies cleanly and everything compiles. However, I am now getting artifacts in the form of large green blocks in my videos (I even try Elephant Dreams as a reference). My videos plays fine if I revert the patch. What could be the problem?

Charles
Comment by Robert Kukla (roolku) - Wednesday, 26 September 2007, 08:04 GMT
I have the same artefacts as cpchan on my gigabeat F with patch_onsvn2007-09-25_r4 and r14857.

In fact I tried to sync the patch myself after Mike Sevakis's dithering commit and got the same problems.
Comment by Brian Morey (Morey) - Wednesday, 26 September 2007, 14:29 GMT
This latest patch was the first attempt to sync with the new dithering commits in the svn. It appears there is an issue causing video playback to be scrambled. I'll post an update as soon as I track down the problem.
Comment by Brian Morey (Morey) - Wednesday, 26 September 2007, 21:23 GMT
This version of the patch is a re-sync to the svn.

The following issues were fixed:
() iPod scroll direction was reversed
() video displaying green blocks
Comment by Nick Waterton (Nick_W) - Wednesday, 26 September 2007, 22:22 GMT
Great, thanks for the quick fix!
Comment by Robert Kukla (roolku) - Thursday, 27 September 2007, 23:48 GMT
Thank you. It also works on iriver h120 and iaudio x5, however the keymaps are incorrect/missing. Please replace

#if (CONFIG_KEYPAD == IRIVER_H100_PAD) || (CONFIG_KEYPAD == IRIVER_H300_PAD)
#define MPEG_SELECT BUTTON_PLAY
#define MPEG_RIGHT BUTTON_RIGHT
#define MPEG_LEFT BUTTON_LEFT
#define MPEG_SCROLL_DOWN BUTTON_SCROLL_UP
#define MPEG_SCROLL_UP BUTTON_SCROLL_DOWN
#define MPEG_EXIT BUTTON_POWER

with

#if (CONFIG_KEYPAD == IRIVER_H100_PAD) || (CONFIG_KEYPAD == IRIVER_H300_PAD)
#define MPEG_SELECT BUTTON_ON
#define MPEG_RIGHT BUTTON_RIGHT
#define MPEG_LEFT BUTTON_LEFT
#define MPEG_SCROLL_DOWN BUTTON_UP
#define MPEG_SCROLL_UP BUTTON_DOWN
#define MPEG_EXIT BUTTON_OFF

#elif (CONFIG_KEYPAD == IAUDIO_X5M5_PAD)
#define MPEG_SELECT BUTTON_PLAY
#define MPEG_RIGHT BUTTON_RIGHT
#define MPEG_LEFT BUTTON_LEFT
#define MPEG_SCROLL_DOWN BUTTON_UP
#define MPEG_SCROLL_UP BUTTON_DOWN
#define MPEG_EXIT BUTTON_POWER
Comment by Brian Morey (Morey) - Friday, 28 September 2007, 00:51 GMT
Thanks for the heads up. This is the first feedback we've received for the iriver player.

The following issues were fixed:
() button mapping for the iriver player

The following features were added:
() support for the iaudio player

Note: If you find this patch useful please click on the vote link at the top of this thread. We put a lot of effort into this patch and this gives us some feedback that people like what we are doing.
Comment by Victor (BukTop) - Friday, 28 September 2007, 06:52 GMT
Great work! Everything works fiine on my sansa e200.Thanks for your hard work.
But why it can't be commited to SVN?
Comment by Victor (BukTop) - Friday, 28 September 2007, 08:54 GMT
Great work! Everything works fiine on my sansa e200.Thanks for your hard work.
But why it can't be commited to SVN?
Comment by Robert Kukla (roolku) - Friday, 28 September 2007, 11:45 GMT
Out of sync again. This is getting frustrating. If you fix it, I will see if I can get it committed over the weekend. Cheers.
Comment by Thomas Martitz (kugel.) - Friday, 28 September 2007, 13:43 GMT
You commit this? Without seeking?
Comment by Robert Kukla (roolku) - Friday, 28 September 2007, 13:48 GMT
KugeL: I don't understand what you are asking - could you elaborate? (you can send me a pm in the forums so we don't swamp the tracker)
Comment by Thomas Martitz (kugel.) - Friday, 28 September 2007, 13:58 GMT
Ah nothing, I just didn't thought that it can be committed. For me, this patch doesn't solve the problems, but works around them (like resume using a data file). Also, I miss seeking most. This patch doesn't really "fix" things in imho.

But I don't have a problem with this patch, I like the features.
Comment by John Gwynne (jgwynne) - Friday, 28 September 2007, 18:38 GMT
Does not do seeking??? I can only imagine by this comment that by seeking you mean some type of fast forward or skipping similar to what one finds on the audio side of the player.

Skip forward:

Implementing that type of functionality for mpegplayer will most likely not be possible for many targets such as the Sansa e200. The reason being the relatively slow processor. We have spent a lot of time studying and testing ideas to do just that and have settled on what you find in the patch. To advance, say 30 seconds, into a stream you have basically one of two choices (that I know of).

(1) Decode, stay synchronous to the stream, not display/play the contents, and quickly move to the point of continuation. The quickly part is the problem. Just decoding the stream, again for the Sansa, is somewhere near 20 frames/sec. Skipping 30 seconds will take on the order of 20 seconds. Not very practical.

(2) Estimate and seek forward to the point in the stream that play would resume, re-synchronize the stream, test the PTS and correct the the stream seek position, once close re-synchronize the audio and video decoders, drop data until audio and video are synchronized. This is the approach used in our patch. On the Sansa with our algorithm this can take from 4-10 seconds based on the accuracy of the estimator. This is not practical for a forward skip button. If you want to skip forward say 5 minutes and the skip button only goes 30 seconds each push, that will take you 100 seconds to advance.

Surely, you must find the slider and preview more useful than something like that?

Fast forward:

Until the frame rate of decoding can be substantially increased, fast forward on targets like the Sansa will only be about 15% faster than normal play. Buffer filling and decoding just take too much CPU power.

Undoubtedly, targets with fast processor and future higher performance players will be able to implement a fast forward or skip function as suggested. But this patch, IMHO, is about the only means that slow targets have of watching videos.

So, if you feel that this patch is not a real “fix”, I will ask if you are naive of the issues? If you know of a better approaches, I would gladly like to hear them.

On a side note, the patch not only adds seeking/resume but it also includes several fixes to coding and implementation errors. The player straight out of svn will crash on average about 1 hour and 20 minutes into a movie. I have watched a number of movies with my Sansa now after patching and know of no problems. mpegplayer really needs some coding help. Developers, will you let us help you?

john
Comment by Thomas Martitz (kugel.) - Friday, 28 September 2007, 20:11 GMT
Chill, I never said that I don't like the start time feature. I'd like to have both :)

I just thought it's doable, because the e200 OF is also able to seek (diffenrent codec though).
Comment by Charles Philip Chan (cpchan) - Friday, 28 September 2007, 20:20 GMT
> I just thought it's doable, because the e200 OF is also able to seek (diffenrent codec though).

The MJPEG codec is decoded in hardware (part of the PortalPlayer chip).

Charles
Comment by Jacob Brooks (jac0b) - Saturday, 29 September 2007, 20:52 GMT
I was trying to get it resynced but it too much for me. It looks like mpegplayer.c had a big change to it and a little to alloc.c, Hope it just fixed fantastic plugin.
Comment by Victor (BukTop) - Saturday, 29 September 2007, 22:05 GMT
But can this patch be modified a little bit to access time seek menu when video is played (I meen to add to mpegplayer menu option "go to specific time"). Because to go to another time you have to exit the playr and launch it again, just to access time seek.
Comment by Charles Philip Chan (cpchan) - Sunday, 30 September 2007, 00:36 GMT
> I was trying to get it resynced but it too much for me. It looks like mpegplayer.c had a big change to it and a little to alloc.c, Hope it just fixed fantastic plugin.

Same here, I manage to get it compiled, but I got a plugin error.

Charles
Comment by Jack Suter (chrisjs169) - Monday, 01 October 2007, 02:55 GMT
Tried running it in a Sim, since it would give more debugging info. In the terminal, the only thing that appears before it dies is "mpeg2_malloc(137784368,2097152,0)" which seems to be what the problem is.
Comment by Thomas Martitz (kugel.) - Monday, 01 October 2007, 08:21 GMT
Before committing, please add the start time seek in the mpeg player menu (so that you don't have to quit the video to set a new start time).
Comment by Brian Morey (Morey) - Monday, 01 October 2007, 12:54 GMT
Hopefully we will have a re-sync soon. This is the second weekend in a row that recent commits have broke this patch. This time it's taking longer to make sure things are put back together in a way that makes sense.

As for start time seeking from the player menu itself, that will be a challenge. We have talked about making that change for some time now, but every time we get around to it we have to spend the weekend fixing the patch due to recent commits.

It might be better to get the current version of the patch committed so that people making commits to SVN can't break the patch and then we can start a new thread adding seek from the play menu.

Anyway, updated patch should be posted as soon as it passes all of our tests.

Note: Thanks for the votes and feedback, it's nice to see people are using our work.
Comment by Brian Morey (Morey) - Monday, 01 October 2007, 20:25 GMT
Re-sync with SVN.
Comment by Jack Suter (chrisjs169) - Monday, 01 October 2007, 22:05 GMT
Brian - Nice, you beat me ;) How'd you get it working?

Sim seems to not return any errors, which is good.
Comment by Robert Kukla (roolku) - Monday, 01 October 2007, 22:47 GMT
I have changed a few tabs with spaces (rockbox style guidelines) and re-instated the key definitions for h120 and x5 which somehow got lost. The progress bar doesn't show on X5 - not sure if this is new or if I didn't notice the last time. For h120 the preview is not updated. It only shows when you press play for a brief moment, before the full screen plays.

Also with the 0.5 min steps, it is a little slow to seek through large files - maybe up/down could be mapped to 10min steps?

I also had a chat with linuxstb (original author) about the patch on irc (see http://www.rockbox.org/irc/reader.pl?date=20071001 from around 23.13) He recons that I should consult with jhMikeS who has plans to implement seeking, so I'll try to catch him.

Linuxstb also mentioned the hardcoded bitmap. Could you perhaps change it over to use the build system or alternatively leave it out completely? A rectangle would be sufficient and it isn't displayed anyway for x5.

Anyway. Thanks for getting it in sync again.
Comment by Marcoen Hirschberg (marcoen) - Tuesday, 02 October 2007, 09:26 GMT
Can you guys working on this patch come to #rockbox on freenode to talk about the future of mpegplayer?
Comment by Robert Kukla (roolku) - Tuesday, 02 October 2007, 09:33 GMT
Okay, I talked to jhMikeS this morning (http://www.rockbox.org/irc/reader.pl?date=20071002 from 10:39 ) and basically got his goahead. He suggested to have the options menu accessible from the start menu, which I think should be possible. If you could have a look at the issues mentioned in my last post - I think in the case of the h120 your loading splash interferes with the preview greyscale mode? Cheers
Comment by Brian Morey (Morey) - Tuesday, 02 October 2007, 13:32 GMT
What do you mean by use the build system, when you talk about the rockbox logo that I hard coded into mpeg_settings.c?
Comment by Thomas Martitz (kugel.) - Tuesday, 02 October 2007, 13:40 GMT
Maybe he wants to throw a bitmap into a specific folder in the tree, which will be used in mpeg player after compiling (just like the splash screen).
Comment by Robert Kukla (roolku) - Tuesday, 02 October 2007, 13:59 GMT
The 'correct' way of dealing with bitmaps is to provide the *.bmp file and let the build system take of the creation of the *.c and *.h files which can then be linked with the code. This makes it easier to change the bitmaps but also to generate the different formats required by different targets. I thought there was a wiki page describing it, but I can't seem to find it. Just look at one of the other plugins that use external bitmaps such as sudoku. Not sure it is worth it though...
Comment by Brian Morey (Morey) - Tuesday, 02 October 2007, 20:05 GMT
(roolku) Thanks for all of your help.

The following changes were made:
() added button mapping for Sansa C200
() Gigabeat and Sansa E200 now skip 10 min. seek on button up/down
() embedded bitmap removed
() code clean-up
Comment by Brian Morey (Morey) - Tuesday, 02 October 2007, 20:38 GMT
Well, svn r14962 broke it again ... at least it worked for a day. lol
Comment by Robert Kukla (roolku) - Tuesday, 02 October 2007, 20:46 GMT
It seemed to merge okay - still testing. Hang in. :)
Comment by Brian Morey (Morey) - Tuesday, 02 October 2007, 21:18 GMT
Note: roolku, make clean in the tools directory doesn't seem to remove all of the .o files. Might be a bug in the Makefile, I didn't have time to look at it yet, I just manually remove them. (telechips.o and gigabeats.o)
Comment by Brian Morey (Morey) - Tuesday, 02 October 2007, 22:11 GMT
Testing a re-sync now. Should post soon.
Comment by Robert Kukla (roolku) - Tuesday, 02 October 2007, 22:16 GMT
Brian, did you get my emails?
Comment by Brian Morey (Morey) - Tuesday, 02 October 2007, 22:31 GMT
Re-sync with SVN.
Comment by Brian Morey (Morey) - Wednesday, 03 October 2007, 18:51 GMT
Re-sync with SVN.

The following changes were made:
() removed floats to follow Rockbox guidelines
() more code clean-up
Comment by Jacob Brooks (jac0b) - Wednesday, 03 October 2007, 19:43 GMT
Could you add a feature so that when a video is paused and it idles off it bookmarks the position.
If this feature is part of the patch already it is not working for me on my gigabeat build.
Comment by Brian Morey (Morey) - Thursday, 04 October 2007, 13:58 GMT
Sorry, that last patch was the wrong file. It was just a re-sync. Here is the new file.

The following changes were made:
() removed floats from code
() better support for smaller players (ipod nano)
() more code clean-up
Comment by Jack Suter (chrisjs169) - Saturday, 06 October 2007, 19:10 GMT
Guess what? It's broken again. At least it lasted two days...
Comment by Jack Suter (chrisjs169) - Saturday, 06 October 2007, 20:48 GMT
Reverting all changes in mpegplayer.c and reapplying the patch resulted in only four failed hunks. I wasn't sure whether to use buffer_tail or buffer_head in replace of buffer_remaining, but buffer_tail looked like it'd work better, and I haven't noticed any problems. I added comments to where I switched buffer_remaining, if there is a problem.

Since all of the other files in the patch are fine, I'll attach to patches - one with everything, and one of just mpegplayer.c
Comment by Robert Kukla (roolku) - Tuesday, 09 October 2007, 20:35 GMT
synched, tidied up and ready to commit

Loading...