Skip to content

Conversation

@Flawake
Copy link
Contributor

@Flawake Flawake commented Dec 26, 2025

No description provided.

@kjarosh
Copy link
Member

kjarosh commented Dec 26, 2025

While testing I found that get_bytes_total actually is 64 bit instead of 32. But I was unable to fix that.

How did you find that? Have you tried playing a 5 GB sound?

@kjarosh kjarosh added A-avm1 Area: AVM1 (ActionScript 1 & 2) T-compat Type: Compatibility with Flash Player labels Dec 26, 2025
@Flawake
Copy link
Contributor Author

Flawake commented Dec 26, 2025

While testing I found that get_bytes_total actually is 64 bit instead of 32. But I was unable to fix that.

How did you find that? Have you tried playing a 5 GB sound?

Yes, that's exactly how I did it.
I created a 5gb mp3 using this ffmpeg command:

ffmpeg -f lavfi -i anoisesrc=color=white:sample_rate=44100 -t 134218 -c:a libmp3lame -b:a 320k noise.mp3

@kjarosh
Copy link
Member

kjarosh commented Dec 26, 2025

You can add a TODO for the u64 total bytes stuff.

@Flawake Flawake force-pushed the sound_bytes branch 2 times, most recently from 9676b3e to 721dc10 Compare December 26, 2025 15:59
@Flawake
Copy link
Contributor Author

Flawake commented Dec 26, 2025

I was sleeping I think. My u64 should have been a u32 instead of the other way around. Has been changed in the last commit

@kjarosh
Copy link
Member

kjarosh commented Dec 26, 2025

I was sleeping I think. My u64 should have been a u32 instead of the other way around. Has been changed in the last commit

So it shouldn't be 64-bit?

@Flawake Flawake marked this pull request as draft December 26, 2025 18:52
@Flawake Flawake force-pushed the sound_bytes branch 7 times, most recently from a889f08 to ed45e67 Compare December 28, 2025 15:04
@Flawake Flawake force-pushed the sound_bytes branch 4 times, most recently from 9ea59af to 96bf0fd Compare December 28, 2025 16:23
@Flawake Flawake marked this pull request as ready for review December 28, 2025 16:37
@Flawake Flawake requested a review from kjarosh December 28, 2025 16:37
@Flawake
Copy link
Contributor Author

Flawake commented Dec 28, 2025

I was sleeping I think. My u64 should have been a u32 instead of the other way around. Has been changed in the last commit

So it shouldn't be 64-bit?

No, should be 32 bit. I was sleeping or something I think

}

/// Estimate the length of a local file for a given request.
fn estimate_file_length(&self, request: &Request) -> Option<u32> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not use SuccessResponse::expected_length()?

Copy link
Contributor Author

@Flawake Flawake Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that one is only available from a future and is not guaranteed to be ready.

    Box::pin(async move {
        let fetch = player.lock().unwrap().fetch(request, FetchReason::Other);
        let mut response = fetch.await.map_err(|error| error.error)?;
...

from core->src->loader.rs line 1273

response is inside the async block.

While flash always has the length ready on local files.

s.loadsound("local_filesystem_file.mp3", b_streaming);
trace(s.getBytesTotal());

always immediatly returns the amount of bytes in flash with local files. Ruffle is not guaranteed to do so in the async block with the response.

Or is it possible to get the SuccessResponse::expected_length() outside of the async block?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if the file is not local? Does it block until we get the content length header?

Copy link
Contributor Author

@Flawake Flawake Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, It returns undefined on a non local file. In flash, and in Ruffle with this commit

Copy link
Contributor Author

@Flawake Flawake Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not added a file from a http request in the tests, because I am not sure if we can ensure that it will always be undefined, networking etc... Can we do that without problems?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do that without problems?

I'm not sure how our tests work in this regard, there might be a race condition.

If it returns undefined for remote files, it's fine to return undefined for local files too in this iteration.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it returns undefined for remote files, it's fine to return undefined for local files too in this iteration.

But that's not what the flash player does. The player always returns the size for local files

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you want to return file size synchronously, you'd need to refactor how fetching is done in order to allow supplying this information synchronously. Currently it's not possible because a future is returned that requires an await.

There's also the question of whether it's really done synchronously, or asynchronously but Flash manages to do it before reading the value in AS. We'd need to simulate a delay to reading a file and check whether something blocks in Flash or undefined is returned. I'm not fully convinced it's done synchronously, because Flash was mainly a web technology and it just doesn't make sense to implement it differently for local files compared to remote files.

I feel like the effort involved outweighs the benefits we get from it, that's why I think it's fine to return undefined for now.

@kjarosh kjarosh added the waiting-on-author Waiting on the PR author to make the requested changes label Jan 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-avm1 Area: AVM1 (ActionScript 1 & 2) T-compat Type: Compatibility with Flash Player waiting-on-author Waiting on the PR author to make the requested changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants