about a patch

[Matthias Kretz]

As I said, I'm looking into making phonon-xine work without the setNextSource 
call. I'm confident this is going to work, so we could revert the revert in 
trunk then.

But I also had the idea of introducing Empty or Unset to MediaSource::Type. 
Then Invalid would keep its meaning of doing something wrong in the 
application while Empty/Unset would tell Phonon to actually do something: 
unset the current source. What do you think?

On Monday 24 November 2008 10:55:20 Thierry Bastian wrote:
> Understood.
>
> Then the descision boils down to either keep the current behavior or fix
> the "bug" that it is impossible to queue an ivalid mediasource.
>
> Honestly I don't care too much about either of them but I'm tempted to say
> I'd prefer to fix the bug, especially since I don't really understand why
> this call was there in the first place.
> Do you fear other backends will suffer from this change? If we keep the
> current behavior, we should probably output a warning if one tries to queue
> an invalid media source. If their intention is to release the resources
> they can then call the new 'clear' slot or even connect the 'finished'
> signal to the 'clear' slot.
>
> Thierry
>
>
> -----Original Message-----
> From: Matthias Kretz [mailto:kretz at kde.org]
> Sent: lundi 24 novembre 2008 10:49
> To: thierry.bastian at nokia.com
> Cc: phonon-backends at kde.org
> Subject: Re: about a patch
>
> Hi Thierry,
>
> I had to revert it because it broke behaviour compatibility, resulting in
> phonon-xine not emitting finished() anymore.
>
> The reason that backend needs this call is that it works completely
> asynchronously, so when it emits aboutToFinish it needs to get some answer
> whether another MediaSource from the queue is to be used or not. With the
> setNextSource call removed, phonon-xine was waiting forever for an answer,
> not
> emitting finished as it didn't know whether something else was to be
> played.
>
> I think I can change phonon-xine to also work right with the call removed,
> but
> then at least the revert in the branch has to stay (don't want to break
> behavior compat in the branch).
>
> On Monday 24 November 2008 09:54:56 thierry.bastian at nokia.com wrote:
> > Hi Matthias,
> >
> > I just saw that you reverted a patch we submitted last week, namely this:
> >
> > @@ -422,6 +422,7 @@ void MediaObjectPrivate::_k_aboutToFinish()
> >      if (sourceQueue.isEmpty()) {
> >          emit q->aboutToFinish();
> >          if (sourceQueue.isEmpty()) {
> > +            pINTERFACE_CALL(setNextSource(MediaSource()));
> >              return;
> >          }
> >      }
> >
> > I was wondering why...
> > The idea behind removing that is that one can call
> > setMediaSource(MediaSource()) and it will unload the current source. To
> > make it consistent with the way queuing works you could imagine enqueuing
> > an invalid mediaSource to unload the last source when it is finished
> > playing.
>
> OK, I didn't know your motivation behind it. And as it broke some parts of
> KDE
> (when using the xine backend) quite badly I had to do a quick revert.
>
> > On the backend side we then need to know if the user/programmer
> > explicitly has queued an invalid media source. Applying this patch makes
> > that this information is no more available. Plus I wonder why it should
> > do that. Any specific reason for it? Our backends don't need that.
> >
> > Thanks for any insight.
> >
> > Regards,
> > Thierry

-- 
________________________________________________________
Matthias Kretz (Germany)                            <><
http://Vir.homelinux.org/