[MPlayer-dev-eng] New patch for play DVD's with dvdnav://

jaime.gemelo at free.fr jaime.gemelo at free.fr
Tue Feb 16 22:14:19 CET 2010


Hi everybody!

Carl Eugen writes:
>> <jaime.gemelo <at> free.fr> writes:
>>
>> > Here is a new version of the "04_dvdnav.patch"
>>
>> Thank you for working on this!

Thanks.

>>
>> Just a few cosmetic comments, somebody else will hopefully do a complete review:
>>
>> [...]
>>
>> > @@ -240,10 +250,16 @@
>> >        case DVDNAV_VTS_CHANGE: {
>> >          priv->state &= ~NAV_FLAG_WAIT_SKIP;
>> >          priv->state |= NAV_FLAG_STREAM_CHANGE;
>> > +		priv->state |= NAV_FLAG_AUDIO_CHANGE;
>>
>> Please do not mix the (existing) spaces with (your) tabs.

It's not an important mistake... So please, don't send this type of "error".

>> @@ -946,15 +1032,9 @@
>>   * \return 1 if audio has changed
>>   */
>>  int mp_dvdnav_audio_has_changed (stream_t *stream, int clear) {
>> -  dvdnav_priv_t *priv = stream->priv;
>> -
>> -  if (!(priv->state & NAV_FLAG_AUDIO_CHANGE))
>> -    return 0;
>> -  if (clear)
>> -    priv->state &= ~NAV_FLAG_AUDIO_CHANGE;
>> -
>> -  return 1;
>> +  dvdnav_priv_t * priv = stream->priv;
>> +  if(!(priv->state & NAV_FLAG_AUDIO_CHANGE)) return 0;
>> +  if(clear) return 1;
>>
>> Please remove all cosmetic changes from this hunk (1 line removed, 1 added, if I
>> see that correct)

Why ?

>>          break;
>>        }
>>        case DVDNAV_SPU_STREAM_CHANGE: {
>>          priv->state |= NAV_FLAG_STREAM_CHANGE;
>> +        priv->state |= NAV_FLAG_STREAM_CHANGE;
>>
>> Duplicated line?
>>
>> [...]

DON'T DELETE THIS LINE! DVDs had problems to change the spu stream.
That's why i do this.
Now I meet no problems with commercial DVDs that i have (Star Wars series, Matrix series,
The Lord of the Rings series, Gun Frontier, Cowboy Bebop, Final Fantasy Advent Children,
etc.).

Carl Eugen wrote (source:
http://lists-archives.org/mplayer-dev-eng/28391-dvdnav-support-improve-change-audio-and-subtitle-by-dvd-menu-and-other-things.html):
>> Just two cosmetic comments:
>> There is one occurrence of trailing white space in the patch - please remove it.
>> And I believe
>>   if (A)
>>     if (B)
>>       C();
>> should be written as
>>   if (A && B)
>>     C();
>>
>> Carl Eugen
It isn't always true. I have meet some problems in some conditions.

>> > Here is a patch that  resolves some things ;) :
>>
>> One patch on this list is expected to fix/implement exactly one thing:
>> Some things => some patches
>>
>> [...]
>>
>> > To apply this patch, launch this command: patch -p1 -i "name of the patch"
>>
>> While everybody here is expected to know how to use patch, please prepare your
>> patches by using "svn diff" - we believe everything else is unreadable.

You're not God. And all people (not only developpers) can visit this page in the aim to
find solutions of his problems.
Thus, for these persons, I decided to explain how they can apply a patch.
And I use this command to create my patch:
diff -abur directory_files_nochange directory_files_modified > mypatch.patch

In a friendly manner!

IMBERT Jacques-Olivier (http://jaime.gemelo.free.fr/)
Java MPlayer project: http://jaime.gemelo.free.fr/javamplayer/index.html




More information about the MPlayer-dev-eng mailing list