Handle EINTR and remove offset_t in ffmpeg code.

Handle EINTR and remove offset_t in ffmpeg code.

[agl, 2010-05-19 16:01:51]

yep, Windows seems happy with this.

[Evan Martin, 2010-05-19 16:04:29]

LGTM

Might be good to check the blame and CC the person behind this code as it goes
in

[wtc, 2010-05-19 17:20:42]

http://codereview.chromium.org/2136017/diff/1/3
File media/ffmpeg/file_protocol.cc (right):

http://codereview.chromium.org/2136017/diff/1/3#newcode59
media/ffmpeg/file_protocol.cc:59: return lseek(GetHandle(h),
static_cast<off_t>(offset), whence);
MSVC has _lseeki64, which takes a 64-bit offset argument:
http://msdn.microsoft.com/en-us/library/1yee101t(VS.80).aspx

Should we use _lseeki64 instead to avoid truncating the
'offset' argument?

[agl, 2010-05-19 18:07:32]

http://codereview.chromium.org/2136017/diff/1/3
File media/ffmpeg/file_protocol.cc (right):

http://codereview.chromium.org/2136017/diff/1/3#newcode59
media/ffmpeg/file_protocol.cc:59: return lseek(GetHandle(h),
static_cast<off_t>(offset), whence);
On 2010/05/19 17:20:42, wtc wrote:
> Should we use _lseeki64 instead to avoid truncating the
> 'offset' argument?

I figured that off_t would be 64-bit on Windows, but the page you cited suggests
not. I've changed it to _lseeki64 on Windows and will land if it passes the try
bot.

[awong, 2010-05-19 18:12:55]

http://codereview.chromium.org/2136017/diff/1/3
File media/ffmpeg/file_protocol.cc (right):

http://codereview.chromium.org/2136017/diff/1/3#newcode59
media/ffmpeg/file_protocol.cc:59: return lseek(GetHandle(h),
static_cast<off_t>(offset), whence);
On 2010/05/19 18:07:32, agl wrote:
> On 2010/05/19 17:20:42, wtc wrote:
> > Should we use _lseeki64 instead to avoid truncating the
> > 'offset' argument?
> 
> I figured that off_t would be 64-bit on Windows, but the page you cited
suggests
> not. I've changed it to _lseeki64 on Windows and will land if it passes the
try
> bot.

Doesn't off_t change on glibc depending on the various features that are
defined?  IIRC, You need _FILE_OFFSET_BITS == 64 right?

[agl, 2010-05-19 18:17:58]

On Wed, May 19, 2010 at 2:12 PM,  <ajwong@chromium.org> wrote:
> Doesn't off_t change on glibc depending on the various features that are
> defined?  IIRC, You need _FILE_OFFSET_BITS == 64 right?

Yes, and we set it in common.gypi.


AGL

[awong, 2010-05-19 18:21:31]

LGTM

On 2010/05/19 18:12:55, awong wrote:
> http://codereview.chromium.org/2136017/diff/1/3
> File media/ffmpeg/file_protocol.cc (right):
> 
> http://codereview.chromium.org/2136017/diff/1/3#newcode59
> media/ffmpeg/file_protocol.cc:59: return lseek(GetHandle(h),
> static_cast<off_t>(offset), whence);
> On 2010/05/19 18:07:32, agl wrote:
> > On 2010/05/19 17:20:42, wtc wrote:
> > > Should we use _lseeki64 instead to avoid truncating the
> > > 'offset' argument?
> > 
> > I figured that off_t would be 64-bit on Windows, but the page you cited
> suggests
> > not. I've changed it to _lseeki64 on Windows and will land if it passes the
> try
> > bot.
> 
> Doesn't off_t change on glibc depending on the various features that are
> defined?  IIRC, You need _FILE_OFFSET_BITS == 64 right?

Ah, cool.

IIRC, scherkus wrote this initially, but I can LG from the media side of things.

Thanks for fixing things.

[wtc, 2010-05-19 18:44:55]

http://codereview.chromium.org/2136017/diff/1/3
File media/ffmpeg/file_protocol.cc (right):

http://codereview.chromium.org/2136017/diff/1/3#newcode59
media/ffmpeg/file_protocol.cc:59: return lseek(GetHandle(h),
static_cast<off_t>(offset), whence);
We can add a static assertion to assert that sizeof(off_t) == 8.