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

Large file read/write block size corrupts data. #1103

Closed
kallanreed opened this issue Jun 3, 2023 · 4 comments
Closed

Large file read/write block size corrupts data. #1103

kallanreed opened this issue Jun 3, 2023 · 4 comments
Labels
bug Something isn't working

Comments

@kallanreed
Copy link
Contributor

There's a comment in PNGWriter.cpp indicating working around a file write problem.

    // Small writes to avoid some sort of large-transfer plus block
    // boundary FatFs or SDC driver bug?

While writing the screenshot viewer, I also ran into this problem if I tried to read a block of 720 bytes all at once. The data was mostly correct, but there was clearly corruption in the data (visible when rendered). I also worked around it by reading in chunks of 240 bytes.

I have not investigated too deeply, but there's likely a bug in FatFs (less likely), or the SD Card driver, or perhaps there's a documented size limit that we need to respect.

@kallanreed kallanreed added the bug Something isn't working label Jun 3, 2023
@gullradriel
Copy link
Member

A limit sounds like a valid explanation, you go over and read garbage

@gullradriel
Copy link
Member

I think according to https://github.com/eried/portapack-mayhem/blob/92072b4225f4f2fae95a60e40fc8b21a36352ebe/firmware/common/ffconf.h#L158 its not a bug.

Documentations about these variables and a quick look at them in the code show that the firmware is configured for a fixed 512 block size . It may work with lower read/write sizes, but no with anything bigger than it.

https://github.com/eried/portapack-mayhem/blob/92072b4225f4f2fae95a60e40fc8b21a36352ebe/firmware/chibios-portapack/ext/fatfs/src/ff.h#L170 indicate that _MAX_SS is also used to configure an internal buffer used in read/write operations.

I think we can close this, but we have to make a Wiki entry about it, in dev section.

@gullradriel
Copy link
Member

And yes, from the look of it 512 seems to be the max block size.

@gullradriel
Copy link
Member

Done added the doc and referenced the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants