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

[Diego Biurrun]

Please get your mailer fixed so that it does not break threading,
thank you.

On Tue, Feb 16, 2010 at 10:14:19PM +0100, jaime.gemelo at free.fr wrote:
> 
> Carl Eugen writes:
> >> <jaime.gemelo <at> free.fr> writes:
> >>
> >> 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".

Whatever the severity, it's a mistake in need of fixing.  Please do not
add tabs to a file that uses spaces exclusively for indentation.

> >> @@ -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 ?

Because it is pointless and makes the patch hard to read.

http://www.mplayerhq.hu/DOCS/tech/patches.txt

> 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.

It is true in C, so it is true for us.

> >> > 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

Please use 'svn diff' instead, it will make your life and our life easier.

Diego