[Harry Muscle]

@Harry
re "There was a rounding error in the edl code that led me to believe that the EDL code isn't accurate enough, but I think it might be. I'll submit a patch to fix the rounding error right away" -> please submit a separate patch for the rounding error. I can't see any change in the trac patch that relates to rounding errors.

Technically it was a truncating error ... but close enough ... take a look here: http://trac.xbmc.org/ticket/10167

Thanks,
Harry

[dteirney]

Technically it was a truncating error ... but close enough ... take a look here: http://trac.xbmc.org/ticket/10167

Thanks,
Harry

Awesome, thanks for the patch. Whoops

[dteirney]

I'm glad we got your attention dteirney ... thanks for all the feedback.

My goal in designing the new file format was to have the most flexibility possible, which is why I wanted to be able to specify the start and end times in pretty much any format possible. Currently I use AVIDemux to figure out the cut points and it provides me with either HH:MM:SS.MMM or frames numbers, both of which needed a manual conversion to be useful in any of the existing supported files (also my frame calculations were never perfect since AVIDemux and XBMC detect ever so slightly different frame rates for the same video). I created the new format with the idea that no matter which video editor the user might be using they will be able to simply copy and paste the info from the video editor into the new EDL file without having to break out the calculator and convert figures. Then I just simply switched from using 0, 1, 2, 3 to something more descriptive.

As for the option of doing only a video blank ... well I'm not 100% sure what the use would be, but again, was going for maximum flexibilty and I figure someone could find a use. The only one I can think of is if there's dialogue in a movie that's important to the story, but it's happening in a child inappropriate place with inappropriate "scenery" in the background.

I'm guessing though that you wanna keep things simpler to avoid confusion, etc. The way I figure, we have a couple of options:

1. Leave things as currently proposed with a new .xdl file format

2. Create a new file with the extension .edlf that's identical to the existing .edl file format but instead of seconds uses frame numbers

3. Create a new file with the extension .edlf that's a combination of the two above ideas. It allows only frames for start and end times, but allows either 0, 1, 2, 3 or a, v, s, c for the action making it somewhat more user friendly to create and not mess up by using the wrong number (or the user can stick with the numbers if they so prefer). Also this would allow the f action to specify the frame rate for the movie in case XBMC detects it wrong. We could forget the av action and simply make v specify both audio and video blank so all actions would be one character to keep more in line with the existing EDL file format. Personally this is as simplified as I would prefer to go since it still allows for a decent amount of flexibility.

4. Don't create a new file extension but instead incorporate the new format ideas into the existing .edl file. So if the file has a 0, 1, 2, 3 action specified treat the start and end fields as the old EDL file format, but if it has one of the new letter actions specified allow full flexibility for start and end times, etc. This would be a good solution for instances where a user might already have existing EDL files, but wants to edit or update then using the new format. The code would allow both types in the same file so the user can simply append to the existing EDL file using the new format.

There's probably another option or two that's possible and if anyone can think of one feel free to propose it. Also anyone who's reading this feel free to comment on which format you would prefer, etc.

Dteirney let me know which one you like, since I think it'll be your call anyway since I'm guessing you'll be the one to approve the patch

Thanks,
Harry

Harry, thanks for your concise analysis of the problems as you see them. Below is my take on what would help you achieve what you need and avoids XBMC EDL support moving too far away from existing "public" EDL file formats (I don't think the Open Source community benefits greatly from another file format and we are already taking some liberties with the .edl file format that MPlayer first introduced).

1) Expand the existing .edl processing logic to support the time code being in HH:MM:SS.sss format. This is easy to do by checking for ":" within the read in string prior to the whitespace and then the code. I think allowing the frame markers to be used is problematic due to the conversion that is necessary to time. Getting the fps from the file can be dubious as you have seen, and extending the format to allow the frame rate to be specified feels like it is pushing the file format too far away from the simple time code and action approach. If you are using AVIDemux and can easily get the time in a formatted way, that will be much more robust.
2) Put that time parsing from "string" code somewhere in an XBMC utility - there might some code that does this already. Perhaps extend / improve StringUtils::TimeStringToSeconds
3) Leave the existing 0, 1, 2, 3 codes for the moment rather than introducing "English only" characters. I agree they are not the most intuitive things to use, but introducing a character is massaging the .edl file format perhaps too far away from the original model.

Future:
4) Implement the Audio Muting / Silence functionality - no idea how, you'll have to get some input from the other devs.
5) Figure out how to integrate with the XBMC Video Bookmarks functionality for Scene Markers that are loaded from an EDL source.
6) Enhance the Commercial Break related EDL sources to automatically include Scene Markers at the end of each commercial break.

Let me know what you think. I feel like I'm acting in a bit of a custodial role, making sure people can do what they need without straying too far from the original EDL intentions and reasonably well known file format.

[dteirney]

re "For short skips it might take longer to seek to the end of the skip than to just play the frames (ie: a skip of 5 frames for example). So for short skips I would just blank the screen and add a small blurb about what's happening (sort of like what happens when XBMC has to buffer). I also would like to add the ability to skip just video, just audio, or both which would allow editing swearing, etc." -> the EDL support in DVDPlayer already hard drops (discards) frames for short cuts, i.e. < 10 seconds. The seeking should be full frame accurate, e.g. the previous reference frame is found and decoded forward from there to the seek point.

Actually, that 10 second check was taken out when the dts timing and non-flushed seeking change was made. If the seek is short and the audio and video demux point is past the seek duration it will just rip past the "dropped" data. It passed all my acid-test videos so definitely works.

[Harry Muscle]

Harry, thanks for your concise analysis of the problems as you see them. Below is my take on what would help you achieve what you need and avoids XBMC EDL support moving too far away from existing "public" EDL file formats (I don't think the Open Source community benefits greatly from another file format and we are already taking some liberties with the .edl file format that MPlayer first introduced).

1) Expand the existing .edl processing logic to support the time code being in HH:MM:SS.sss format. This is easy to do by checking for ":" within the read in string prior to the whitespace and then the code. I think allowing the frame markers to be used is problematic due to the conversion that is necessary to time. Getting the fps from the file can be dubious as you have seen, and extending the format to allow the frame rate to be specified feels like it is pushing the file format too far away from the simple time code and action approach. If you are using AVIDemux and can easily get the time in a formatted way, that will be much more robust.
2) Put that time parsing from "string" code somewhere in an XBMC utility - there might some code that does this already. Perhaps extend / improve StringUtils::TimeStringToSeconds
3) Leave the existing 0, 1, 2, 3 codes for the moment rather than introducing "English only" characters. I agree they are not the most intuitive things to use, but introducing a character is massaging the .edl file format perhaps too far away from the original model.

Future:
4) Implement the Audio Muting / Silence functionality - no idea how, you'll have to get some input from the other devs.
5) Figure out how to integrate with the XBMC Video Bookmarks functionality for Scene Markers that are loaded from an EDL source.
6) Enhance the Commercial Break related EDL sources to automatically include Scene Markers at the end of each commercial break.

Let me know what you think. I feel like I'm acting in a bit of a custodial role, making sure people can do what they need without straying too far from the original EDL intentions and reasonably well known file format.

I'm fine with all the suggestions you mention except for one

I really would like to add the ability to specify frame numbers into the file format. To be honest it's sort of what's motivating me to do all this work on the EDL, I really would like to get frame accurate EDL and you can't do that without being able to specify frames. I've tried to achieve frame accuracy by specifying times from AVIDemux, however, XBMC and AVIDemux seem to calculate slightly different frame rates for the same video files, so for example what might be time index 1:22:33.444 in AVIDemux turns out to be 1:22:33.567 in XBMC, which ends up making a difference of a few frames. The difference seems to come from the fact that XBMC calculates the frame rate upto at least 6 decimal places (or more possibly) and I think AVIDemux truncates at three, which a good hour into the movie starts to make a difference of a few frames. Chances are other video editors would prove similarly problematic with their times not matching perfectly with XBMC.

Ironically specifying the frame number proves more accurate than specifying the time. I think you're not giving enough credit to how accurate the frame to time conversion turns out to be in XBMC. I've done a whole bunch of tests with my patch and specifying frame numbers and every single time except for one I got perfect frame accuracy. The one time it was off by one frame. Not bad I would say.

So I was thinking in addition to the points you made, what if we allow the user to specify the frame number. At first I though maybe add the letter "f" to the end of the number, but then that would make it english again. So I though what if we precede the number with "#". So to specify frame 1234, you would write #1234. I think it make sense, since the # symbol already means "number" in most languages (if not all).

Let me know what you think of adding this item to your list of suggestions.

Thanks,
Harry

[dteirney]

Ironically specifying the frame number proves more accurate than specifying the time. I think you're not giving enough credit to how accurate the frame to time conversion turns out to be in XBMC. I've done a whole bunch of tests with my patch and specifying frame numbers and every single time except for one I got perfect frame accuracy. The one time it was off by one frame. Not bad I would say.

Interesting stuff re the detected fps. There isn't a single format that I know of that specifies the fps being to more than 3 decimal places. 23.976 from http://en.wikipedia.org/wiki/Frame_rate

Most are only to 2 decimal places. What happens if you truncate the FPS passed in from XBMC to the EDL area to 3 decimal places. Does everything line up well then?

[dteirney]

Ironically specifying the frame number proves more accurate than specifying the time. I think you're not giving enough credit to how accurate the frame to time conversion turns out to be in XBMC. I've done a whole bunch of tests with my patch and specifying frame numbers and every single time except for one I got perfect frame accuracy. The one time it was off by one frame. Not bad I would say.

So I was thinking in addition to the points you made, what if we allow the user to specify the frame number. At first I though maybe add the letter "f" to the end of the number, but then that would make it english again. So I though what if we precede the number with "#". So to specify frame 1234, you would write #1234. I think it make sense, since the # symbol already means "number" in most languages (if not all).

Internally everything is still time based within XBMC, and for frame to time conversion the frame rate is still needed. If rounding XBMC detected frame rate to 3dp works I guess that would be preferable to having to specify it in the file. I'm not sure how the dts timestamps within the streams relate to the precision of the frame rate.

Might need to massage the EDL frame to time conversion logic to bump back or forward a small amount at the edges for frame perfect playback. I.e. is the frame specified as the start of the cut included in the cut, or excluded from the cut - similar for the end of the cut.

[dteirney]

So I was thinking in addition to the points you made, what if we allow the user to specify the frame number. At first I though maybe add the letter "f" to the end of the number, but then that would make it english again. So I though what if we precede the number with "#". So to specify frame 1234, you would write #1234. I think it make sense, since the # symbol already means "number" in most languages (if not all).

I think this makes sense if there is still some way to get an "accurate" fps reading from XBMC. Let me know what you find out if truncating the detected fps to 3dp helps get your frame number based skipping to match what you are seeing in AVIDemux.

[Harry Muscle]

Internally everything is still time based within XBMC, and for frame to time conversion the frame rate is still needed. If rounding XBMC detected frame rate to 3dp works I guess that would be preferable to having to specify it in the file. I'm not sure how the dts timestamps within the streams relate to the precision of the frame rate.

Might need to massage the EDL frame to time conversion logic to bump back or forward a small amount at the edges for frame perfect playback. I.e. is the frame specified as the start of the cut included in the cut, or excluded from the cut - similar for the end of the cut.

I would think that truncating the frame rate to 3dp for the calculations would still not provide the perfection that's possible from allowing the user to specify frames. Having though more about this, I'm thinking the reason why XBMC might have such high accuracy for the frame rate information is because XBMC displays the video at slightly different frame rates in order to match up with the refresh rates of the display device (can't remember what the feature is called of the top of my head). That would also explain why XBMC and AVIDemux don't match up on frame rates. Maybe they do, but XBMC modifies the frame rate to match the display refresh rate, so the times don't match up to the same frames.

That would mean that the only way to get the time of the frames you wanna cut is to rely on XBMC to do the math for us using the frame rate information that XBMC will be using to display the movie. Which is basically what it can do now if we just allow a format that specifies frames.

Thanks,
Harry

[dteirney]

That would mean that the only way to get the time of the frames you wanna cut is to rely on XBMC to do the math for us using the frame rate information that XBMC will be using to display the movie. Which is basically what it can do now if we just allow a format that specifies frames.

There isn't anything in the EDL functionality that detects a frame number and then skips based on that. The cut or automatic seek detection for all of the EDL file formats is done based on the dts time for the playback. All of the EDL files that support frame markers are converted to times using the detected frame rate of the video.

As far as I know the time presented on screen is still based on the dts timing information not the strict wall time as might be shown if a stopwatch was started when playback started.

It is possibly pure co-incidence that seeking based on frame number is working better than timing, because they should behave identically if the same fps is used to convert between the frame number and the time.

[outleradam]

Maybe you could fix the descrepencies between comskip 1 and xbmc format. MythicalLibrarian generates true comskip 1 files, but XBMC puts the cuts in the wrong spots.

[dteirney]

Maybe you could fix the descrepencies between comskip 1 and xbmc format. MythicalLibrarian generates true comskip 1 files, but XBMC puts the cuts in the wrong spots.

Trac ticket? More information?

[outleradam]

It seems to only affect comskip 1 format http://forum.xbmc.org/showthread.php?tid=80481

[Harry Muscle]

SNIP

As far as I know the time presented on screen is still based on the dts timing information not the strict wall time as might be shown if a stopwatch was started when playback started.

SNIP

Hmm, then I got no clue why XBMC would maintain such high accuracy of the frame rate.

Nonetheless, I tried the idea of truncating the frame rate down to 3 decimal places, but unfortunately the times from AVIDemux still don't match up perfectly to the same frame in XBMC.

So if you don't mind, I'll put together a patch that incorporates all the ideas from your post yesterday (post #33) and add the support for specifying frame numbers using the # symbol.

I should have a bit more time to work on this Monday or Tuesday so I'll post a new patch to the trac ticket then and update things on here.

Thanks for all the feedback, etc. Much appreciated to have another developer to bounce this stuff off of.

Thanks,
Harry

[pecinko]

fuck me, hell if i know.

And what if he starts with forums first?