Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Even better emulate POSIX file functions #5694

Closed
wants to merge 1 commit into from

Conversation

Karlson2k
Copy link
Member

Properly support feof() and ferror() emulated functions.

err != ENOBUFS && err != ENOMEM && err != ENXIO))
errno = EIO; // exact errno is unknown or incorrect, use default error number
}
return ret;

This comment was marked as spam.

This comment was marked as spam.

@MilhouseVH
Copy link
Contributor

Video playback is still not working in all cases.

Try the sample VIDEO_TS folders here.

The "NOT_OK" folder does not play (testing over SMB) when Karlson2k@e502b09 is included, and it also does not play when this PR is included (in addition to Karlson2k@e502b09).

debug log playing NOT_OK over SMB with a build based on master with PR5694+e502b09. (full log).

@Karlson2k
Copy link
Member Author

Seems that some external lib doesn't handle correctly partial read.
Will test it on Ubuntu.

@MilhouseVH
Copy link
Contributor

I added a debug log to my previous comment in case that is helpful to you.

@Karlson2k
Copy link
Member Author

@MilhouseVH Thanks! Could you do the same with PR5694-e502b09?

@MilhouseVH
Copy link
Contributor

Yes, PR5694-e502b09 is working fine (it's always fine when e502b09 is reverted! :))

debug log playing back NOT_OK over SMB with PR5694-e502b09, and the full log.

@Karlson2k
Copy link
Member Author

@MilhouseVH We always could revert before release. I want to find real root problem. Thanks for testing and log.

@MilhouseVH
Copy link
Contributor

Yes, I think it will be necessary to revert e502b09 unless the root problem can be identified and fixed fairly quickly.

This is a problem for all platforms so reverting e502b09 on master before the next round of betas might be a good idea, then continue testing yourself with e502b09 added (the VIDEO_TS is a reproducible test case for me, at least on Pi/OpenELEC/SMB) until the problem is fixed?

@Karlson2k
Copy link
Member Author

@MilhouseVH Looks like I found the root problem for broken reading.
We are using libdvdread with libdvdcss. In that case all reading comes through dvdcss_read().
At https://github.com/xbmc/xbmc/blob/master/lib/libdvd/libdvdcss/src/libdvdcss.c#L655 patrial read is not supported: pf_read() maps to libc_read() and it simply return read data (libc_read() support for partrial read look fully broken, it use libc_seek() with relative offset while libc_seek() expect absoulte offset).
Without libdvdcss, libdvdnav could use internal file_read() instead of dvdcss_read(). In this case partial reads are correctly supported: https://github.com/xbmc/xbmc/blob/master/lib/libdvd/libdvdread/src/dvd_input.c#L223.
Need to patch libdvdcss.

@Karlson2k
Copy link
Member Author

For fix of broken ifo handling: PR #5700

@Karlson2k Karlson2k added Type: Fix non-breaking change which fixes an issue Helix labels Nov 12, 2014
@topfs2
Copy link
Contributor

topfs2 commented Nov 12, 2014

Big change very late. I feel a bit reluctant at pulling this.

What bugreports does it fix?

@Karlson2k
Copy link
Member Author

It was supposed to fix reading over SMB, but it didn't fix it as we have a bug in libdvdcss.
Exclude "Correctly support feof() and ferror()" and you'll get obvious changes.

@Karlson2k
Copy link
Member Author

Rebased, only last commit.

@stefansaraev
Copy link
Contributor

ping, is this still needed?

@MartijnKaijser
Copy link
Member

If still needed please open a new PR and reference this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants