Arnavion / libjass

Renders ASS subs in the browser.
Apache License 2.0
174 stars 29 forks source link

libjass.deserialize dialogue style changes not work #107

Open jokerrider007 opened 6 years ago

jokerrider007 commented 6 years ago

Previously, I'm ask the way to generate ASS object from JSON object. I have some problem. The problem is when i try to change styles Map object generate from libjass.deserialize. Then call render.resize() style property in dialogue don't change.

var result = libjass.deserialize(JSON.stringify(data));
var renderer = new libjass.renderers.DefaultRenderer(video, result);
renderer.resize(width, height, left, top) ;

var style = renderer.ass.styles.get("Default");
style._fontSize = 80;
renderer.ass.styles.set("Default",style);
renderer.resize(width, height, left, top) ;
cosoole.log(renderer.ass.styles.get("Default")._fontSize ); // fontSize = 80
console.log(renderer.ass.dialogue[0].style._fontSize ); // fontSize = 50

I use libjass.deserialize to create ASS object and render. Then i try to change property in 'Default' styles. when i change _fontSize and call render.resize(). Dialogue use 'Default' style _fontSize not change.

So I tested with libjass.ASS.fromUrl. it's work, when i I change styles Map and call render.resize() dialogue style property has change. I'm very confused. how can i change dialogue style from libjass.deserialize object render?

Arnavion commented 6 years ago

Yes, every Dialogue serializes its own copy of a Style object, so the deserialized Dialogue has an independent Style object unrelated to the one deserialized into ASS.styles.

One way to fix it is to to change serialize() to use an instance toJSON() if it exists (similar to how deserialize uses a static fromJSON() if it exists), and Dialogue should implement toJSON to only serialize the style name and fromJSON to look up the style based on the name.

But that won't actually work, since the ass object doesn't exist when the Dialogue objects are being deserialized, so there's nothing to look up the style from.


The immediate solution is to do something like:

var result = libjass.deserialize(JSON.stringify(data));
for (const dialogue of result.dialogues) {
    dialogue._style = result.styles.get(dialogue.style.name);
}

var renderer = new libjass.renderers.DefaultRenderer(video, result);
/* ... */

to reset the Dialogue.style object to the same one that's in the ASS.styles map.


Also, in

var style = renderer.ass.styles.get("Default");
style._fontSize = 80;
renderer.ass.styles.set("Default",style);

you don't need the third line. Style is an object so mutating the value you get from Map.get() will mutate the value that's in the Map.

Arnavion commented 6 years ago

I'll keep this open since it's a bug. You can unsubscribe from it if you don't want notifications.