Associate sounds with instance ID

[relrelb]

Fixes #1441
Fixes #1319

[Herschel]

Thanks! I wouldn't want to merge this without the sound instances cleaning up after themselves; currently the instance list would "leak" unless sounds are explicitly stopped. Event sounds from the timeline should be stopped as well.

[relrelb]

I came up with a better solution that doesn't leak the sound instances, so this is not a prototype anymore and ready for CR.

[Herschel]

Using Character ID is troublesome because:

• Instances of the same clip share the same character ID, but each instance should stop their own sounds separately
• Movies that load other movies; character IDs are only unique in the same movie. (see Audio streams in different movies with the same character ID will overwrite each other #1319).

One possibility is to add an unique increasing instance_id to DisplayObject. There is already UpdateContext.instance_counter that is used for generating default instance names, we can store this ID in DisplayObject as well.

[relrelb]

Now sounds are stopped per an instance ID, which is the UpdateContext.instance_counter that gets incremented on each DisplayObject instantiation.

However, there are some Rust borrowing issues left, which you might assist me to solve. In short, I made instantiate_display_object in library.rs receive an UpdateContext as parameter rather than a MutationContext. So I changed all instantiate_display_object (directly or indirectly) to pass context rather than context.gc_context. This causes 5 similar errors, for example in movie_clip.rs:

if let Ok(child) = context // 1st borrow
    .library
    library_for_movie_mut(self.movie().unwrap())
    instantiate_by_id(id, context) // 2nd borrow, context used to be context.gc_context
{
    ...

The problem is that context is now mutably borrowed twice. Do you have any idea how to solve it?

[kmeisthax]

You can't chain methods on context variables that also need the context. This is the same reason why Avm1 and Avm2 don't take &self. For libraries, however, the library itself is owned so you can just move it out of the temporary:

let library = context.library.library_for_movie_mut(self.movie().unwrap());
if let Ok(child) = library.instantiate_by_id(id, context) {
   //context and library should both have zero borrows within this block
}

Rust's borrow checker is a little weird with how it accounts for lifetimes and borrows accrued in temporaries; any borrow that's part of an if let statement lasts for the entire block, even if it makes no semantic sense to do so. This even applies when RefCell semantics are at work; you can totally get spurious BorrowMutError panics simply because you accidentally held a temporary in an if let.

[relrelb]

@kmeisthax Thanks for your help, but unfortunately the compiler still complains for the same reason. Can you please try to compile the code yourself and confirm it's working?
After looking for similar code that takes context.library and then uses context, I haven't found any code that uses both library and context in the same statement, as my code does. I suspect this is the difference that makes it difficult to compile.

[Herschel]

You could do it in post_instantiation alongside set_default_instance_name -- I think that's the least invasive change for now.

[relrelb]

@Herschel Thanks for your suggestion! Despite I think it's not the ideal way, I did it in set_default_instance_name (which I renamed to set_instance_id_and_default_instance_name), and it seems to work well.

One thing that still remained - few methods currently accept an optional instance_id (e.g. start_sound), and few places pass None to them. I'm pretty sure all sounds should be associated with some instance_id, but I'm not familiar enough with the Flash Player behavior. I would be thankful if you review my changes and pay extra attention to the TODOs I placed near such cases.

[ousia]

Was this request abandoned? Sorry for asking that, but it seems to be no activity for over a month.

Maybe I’m just being impatient. (I must confess that I’m extremely interested in an improvement for #340, reported almost a year ago).

Many thanks for your excellent work.

[relrelb]

Mostly superseded by #2825, but still needed for #1319?

[relrelb]

Completely superseded by #2825.