'[Common-dev] RE: [Nokia-private-dev] CN: ou1cimx1#419122 - Player'

[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-common-dev
Subject:    [Common-dev] RE: [Nokia-private-dev] CN: ou1cimx1#419122 - Player
From:       <ext-shashi.merapala () nokia ! com>
Date:       2010-10-26 15:21:26
Message-ID: EEF08FFE3AB06D43A1F6BDF1C0F779B33011880A3B () NOK-EUMSG-02 ! mgdnok ! nokia ! com
[Download RAW message or body]

Thanks Jamie for the review. The changes are committed to 210CayS, HEAD, & 420Brizo.
MKV file format does not have a valid response object to inform the read error via \
read done. And, the mini file object informs the file format of the error only via \
the response object. Hence, went ahead with this fix.

Thanks & Warm Regards,
Shashi Kiran Merapala

-----Original Message-----
From: ext Jamie Gordon [mailto:jgordon@real.com] 
Sent: Thursday, October 21, 2010 9:57 PM
To: Merapala Shashi (EXT-Sasken/Bangalore)
Cc: common-dev@helixcommunity.org; filesystem-dev@helixcommunity.org; \
                datatype-dev@helixcommunity.org; nokia-private-dev@helixcommunity.org
Subject: Re: [Nokia-private-dev] CR: ou1cimx1#419122 - Player isn't closed properly \
when connecting phone in Mass Storage mode when playing .mkv video file.

Looks okay, but Read is still always supposed to inform the response
object of error (via ReadDone), regardless whether the underlying call
is synchronous or asynchronous. It should only return an error instead
of informing the response object in case it has no valid response
object.

Thanks,
Jamie

On 10/21/2010 7:00 AM, ext-shashi.merapala@nokia.com wrote:
> Hello Jamie,
> 
> Incorporated the review comments and hence attaching the new diffs.
> Kindly review.
> 
> MKV file format uses 64-bit synchronous read calls for reading from the
> file.
> 
> Hence, the mini file object read in question is a 64-bit sync read call
> which originally returns HXR_OK / HXR_FAIL based on whether data is read
> or not. This read can fail (no data is read) when there is a read error
> or when the file reaches the end (EOF) or with partial playback cases.
> In all these cases, the read call returns HXR_FAIL which is propagated
> as HXR_NO_DATA to MKV file format resulting in calling stream done on
> that particular stream.
> 
> Now, in case of read errors as in this use-case, the 64-bit sync read
> call of mini file object does not inform the file format of the read
> error (as it does with the async read calls where read errors are
> propagated to the file format via read done). It only returns HXR_FAIL
> which is not sufficient to distinguish the nature of the error.
> 
> So, the change is made to explicitly return HXR_READ_ERROR from the mini
> file object in case of a read error so that the MKV file format can
> inform velproxy of the error through PacketReady.
> 
> Thanks & Warm Regards,
> 
> Shashi Kiran Merapala
> 
> -----Original Message-----
> From: ext Jamie Gordon [mailto:jgordon@real.com]
> Sent: Friday, October 08, 2010 11:17 PM
> To: Merapala Shashi (EXT-Sasken/Bangalore)
> Cc: common-dev@helixcommunity.org; filesystem-dev@helixcommunity.org;
> datatype-dev@helixcommunity.org; nokia-private-dev@helixcommunity.org
> Subject: Re: [Nokia-private-dev] CR: ou1cimx1#419122 - Player isn't
> closed properly when connecting phone in Mass Storage mode when playing
> .mkv video file.
> 
> What is the purpose of this new method CheckIfReadError meant to be?
> 
> Read errors should *always* be propagated via ReadDone. (All calls to
> 
> Read must be followed by a call to ReadDone to inform the response
> 
> object of the status of the read!) If the mini file system is not
> 
> propagating some errors then it should be fixed to do so, and if the
> 
> mkvff is not handling some errors in its ReadDone, then it needs to
> 
> be fixed to handle the error there.
> 
> Also, you can't modify released interfaces, as that breaks all sorts
> 
> of plugin compatibility. If a new interface method *is* necessary on
> 
> the file object you need to create a new interface to expose it.
> 
> Thanks,
> 
> Jamie
> 
> On 10/7/2010 10:55 PM, ext-shashi.merapala@nokia.com wrote:
> 
> > "Nokia submits this code under the terms of a commercial contribution
> 
> > agreement with RealNetworks, and I am authorized to contribute this code
> 
> > under said agreement."
> 
> > 
> 
> > Modified by: ext-shashi.merapala@nokia.com
> 
> > <mailto:ext-shashi.merapala@nokia.com>
> 
> > 
> 
> > Reviewed by: shy.ward@nokia.com <mailto:ext-shashi.merapala@nokia.com>
> 
> > 
> 
> > Date: 10/08/2010
> 
> > 
> 
> > Project: SymbianMmf_wm
> 
> > 
> 
> > Error Id: ou1cimx1#419122
> 
> > 
> 
> > Synopsis: Player isn't closed properly when connecting phone in Mass
> 
> > Storage mode when playing .mkv video file.
> 
> > 
> 
> > Overview: While playing any mkv file from the mass storage, if the
> phone is connected to the PC in mass storage mode, read failure happens.
> This read error that occurs during this condition is not being
> propagated to velproxy resulting in the erroneous behavior.
> 
> > 
> 
> > 
> 
> > 
> 
> > Solution: Changes are made to propagate the read error from mini file
> object to mkv file format and subsequently to velproxy so that the
> player closes properly.
> 
> > 
> 
> > Diffs attached.
> 
> > 
> 
> > Files added: None
> 
> > 
> 
> > Files modified: /cvsroot/common/include/hxfiles.h
> 
> > 
> 
> > /cvsroot/filesystem/local/mini/minifileobj.h
> 
> > 
> 
> > /cvsroot/filesystem/local/mini/minifileobj.cpp
> 
> > 
> 
> > /cvsroot/datatype/mkv/fileformat/pub/mkv_file_format.h
> 
> > 
> 
> > /cvsroot/datatype/mkv/fileformat/mkv_file_format.cpp
> 
> > 
> 
> > 
> 
> > 
> 
> > Image size and heap use impact: Negligible
> 
> > 
> 
> > Module release testing (STIF): Done. Passed.
> 
> > 
> 
> > Test case(s) added: No
> 
> > 
> 
> > Memory leak check performed: NA
> 
> > 
> 
> > Platforms and profiles build verified: helix-client-s60-52-mmf-mdf-dsp
> 
> > 
> 
> > Platforms and profiles functionality verified: armv5
> 
> > 
> 
> > Branch: 210CayS, HEAD, 420Brizo
> 
> > 
> 

_______________________________________________
Common-dev mailing list
Common-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/common-dev

[prev in list] [next in list] [prev in thread] [next in thread] 