stop-time < 0 == length - abs(stop-time)

[MooseSoftware]

A feature I would really like in VLC is to be able to pass a negative stop-time parameter and have the video stop playing this many seconds prior to the end of the video. Great for chopping off ending credits, etc.

Example the following command line would stop playing the video 15 seconds before the end of the video:

Code: Select all

vlc "moose.avi" --stop-time -15

Anyway, here's the code to support this:
* Source File: vlc-2.1.5\src\input\input.c
* Method: StartTitle

Insert the following lines of code after line 968:

Code: Select all

if( p_input->p->i_stop < 0 ) { /* Negative stop-time, so calculate real stop-time = video length + stop-time */ p_input->p->i_stop = input_item_GetDuration( p_input->p->p_item ) + p_input->p->i_stop ; }

Hope this makes it into the next release of VLC.

[Jean-Baptiste Kempf]

You should send a patch. And you should also check the return of input_item_GetDuration

[MooseSoftware]

You should send a patch. And you should also check the return of input_item_GetDuration

Yes OK will do. I'm downloading / installing MSysGit, TortoiseGit, etc and I'll try and follow the extremely convoluted process of submitting a patch. Hope I get it right. Probably going to take me a few hours - and that's if all goes really well and the Developer's Corner instructions are clear, correct, and complete. Hope so.

[MooseSoftware]

And you should also check the return of input_item_GetDuration

Yes, I'm a firm believer in always checking the return value of method calls whenever appropriate or required.

However, I have just searched through all 159 MB of VLC source code (3,359 files in 315 folders) and the input_item_GetDuration method is called many times, but the return value of this method is *never* checked by any calling module.

Here are the results of my search:

Code: Select all

vlc-2.1.5\vlc-2.1.5\include\vlc_input_item.h : VLC_API mtime_t input_item_GetDuration( input_item_t * p_i ); vlc-2.1.5\vlc-2.1.5\lib\media.c : return from_mtime(input_item_GetDuration( p_md->p_input_item )); vlc-2.1.5\vlc-2.1.5\modules\control\dbus\dbus.c : dbus_int64_t i_mtime = input_item_GetDuration( p_input ); vlc-2.1.5\vlc-2.1.5\modules\gui\macosx\ControlsBar.m : mtime_t dur = input_item_GetDuration(input_GetItem(p_input)); vlc-2.1.5\vlc-2.1.5\modules\gui\macosx\fspanel.m : mtime_t dur = input_item_GetDuration(input_GetItem(p_input)); vlc-2.1.5\vlc-2.1.5\modules\gui\macosx\playlist.m : mtime_t dur = input_item_GetDuration(p_item->p_input); vlc-2.1.5\vlc-2.1.5\modules\gui\macosx\StringUtility.m : mtime_t dur = input_item_GetDuration(input_GetItem(p_input)); vlc-2.1.5\vlc-2.1.5\modules\gui\qt4\adapters\chromaprint.cpp : mtime_t t = input_item_GetDuration( p_item ); vlc-2.1.5\vlc-2.1.5\modules\gui\qt4\components\playlist\sorting.h : i_duration = input_item_GetDuration( p_item ) / 1000000; vlc-2.1.5\vlc-2.1.5\modules\lua\libs\input.c : mtime_t duration = input_item_GetDuration( vlclua_input_item_get_internal( L ) ); vlc-2.1.5\vlc-2.1.5\modules\misc\audioscrobbler.c : p_sys->p_current_song.i_l = input_item_GetDuration(p_item) / 1000000; vlc-2.1.5\vlc-2.1.5\modules\misc\rtsp.c : p_media->i_length = input_item_GetDuration( p_item ); vlc-2.1.5\vlc-2.1.5\modules\misc\playlist\html.c : mtime_t i_duration = input_item_GetDuration( p_current->p_input ); vlc-2.1.5\vlc-2.1.5\modules\misc\playlist\m3u.c : mtime_t i_duration = input_item_GetDuration( p_current->p_input ); vlc-2.1.5\vlc-2.1.5\modules\misc\playlist\xspf.c : i_duration = input_item_GetDuration( p_item->p_input ); vlc-2.1.5\vlc-2.1.5\modules\stream_out\vod.c : p_media->i_length = input_item_GetDuration( p_item ); vlc-2.1.5\vlc-2.1.5\src\libvlccore.sym : input_item_GetDuration vlc-2.1.5\vlc-2.1.5\src\input\input.c : i_length = input_item_GetDuration( p_input->p->p_item ); vlc-2.1.5\vlc-2.1.5\src\input\item.c : mtime_t input_item_GetDuration( input_item_t *p_i ) vlc-2.1.5\vlc-2.1.5\src\playlist\item.c : mt_duration += input_item_GetDuration( p_input ); vlc-2.1.5\vlc-2.1.5\src\playlist\sort.c : mtime_t time1 = input_item_GetDuration( first->p_input ); mtime_t time2 = input_item_GetDuration( second->p_input ); vlc-2.1.5\vlc-2.1.5\src\text\strings.c : mtime_t i_duration = input_item_GetDuration( p_item ); mtime_t i_duration = input_item_GetDuration( p_item ); [b]Found 25 occurence(s) in 23 file(s).[/b]

I've looked at the code around each of these calls, and none of the methods check the return value of the input_item_GetDuration method.

Also, just a suggestion .... some of the code in VLC could use more meaningful naming conventions (I have looked at a very small proportion of it so far). For example, this is in the GetInputMeta method in the file vlc-2.1.5\modules\control\dbus\dbus.c:

Code: Select all

/** The duration of the track can be expressed in second, milli-seconds and µ-seconds */ dbus_int64_t i_mtime = input_item_GetDuration( p_input ); dbus_uint32_t i_time = i_mtime / 1000000; dbus_int64_t i_length = i_mtime / 1000;

It's only a small module, and the comment is good and makes things clear, so it could be excused. However, far better names for these variables would be something like i_usec_time instead of the confusing i_mtime - which someone might misinterpret as "millisecond time" since it has an "m" in it, and i_msec_time instead of the meaningless i_length, and i_sec_time instead of the meaningless i_time.

Here's the code for the input_item_GetDuration method in vlc-2.1.5\src\playlist\item.c:

Code: Select all

mtime_t input_item_GetDuration( input_item_t *p_i ) { vlc_mutex_lock( &p_i->lock ); mtime_t i_duration = p_i->i_duration; vlc_mutex_unlock( &p_i->lock ); return i_duration; }

So, there is no checking for errors / issues inside the method and therefore there is no return value to indicate an error may have occurred. This explains why no methods check the return value of the input_item_GetDuration method.

OK, now on to getting this patch submitted ....

[MooseSoftware]

OK, patch submitted (I think) - at least the changes were committed.

Some feedback about the process:
(1). The Git Windows (https://wiki.videolan.org/Git_Windows/) guide is extremely incomplete and broken and I just could not get the Windows GIT tools (MSysGit and TortoiseGit) to work properly - they are are best extremely unintuitive and at worst seriously broken.
(2). The Git guide (https://wiki.videolan.org/Git) is much better than the Git Windows page, and I liked using GIT BASH, but the Git guide is not clear in many places and could benefit a lot from adding examples.
(3). Anyhow, I used GIT BASH (love this !) and muddled through the steps and did everything that seemed like it was required, except for not being able to send the email: "git-send-email: command not found". Ooops ..... If I had known this could be an issue at the start, I would have dusted off a Linux VM and done the work in there. Not keen to download 100+ MB of code and go through all this again.
(4). Maybe add a "step by step guide" ? Take someone through the whole process from start to finish with a simple update to a single file.

Anyway, here are the comments I added to the git commit:

Code: Select all

Add support for negative stop-time parameter values to make the media (audio or video) stop playing this many seconds prior to the end. Great for chopping off ending credits, etc. Example the following command line would stop playing the video 15 seconds before the end of the video: vlc "moose.avi" --stop-time -15

I think I was supposed to put "[PATCH]" at the start of the 1st line ??? If so, I apologise (and in any case, the Git guide should have been clearer and had examples).

Some examples in the Git guide (https://wiki.videolan.org/Git) would sure help a lot.

If the patch email is a problem, please let me know and what I can do to fix it.

[Jean-Baptiste Kempf]

However, I have just searched through all 159 MB of VLC source code (3,359 files in 315 folders) and the input_item_GetDuration method is called many times,

That's not correct, but it does not matter. In your case, it's important, because it could be 0 aka, that we don't know the duration, and that would be a failure, because it would get a negative number.