PyonFX icon indicating copy to clipboard operation
PyonFX copied to clipboard

0.9.10 reloaded - load support for file and in memory content

Open sosie-js opened this issue 4 years ago • 3 comments

Hello,

I refactorized the logic adding functions -load(path) : Load file content or content -input(path) ; change the input path -output(path): change the output path -validate(data) : true if data is a valid ass content -add_pyonfx_extension() launch calculations to extend data

and cover security checks, -when no file is given, we default to the Aegisub default Untitled.ass . (Bonus) -when no video is attached, play_res_x or play_res_y are missing from info section, prevents add_pyonfx_extension crash.

Example 0 has been added for demo in Basic Examples. Enjoy!

sosie-js avatar Jun 24 '21 17:06 sosie-js

Thank you very much for your PR! Reading the changelog only, everything sound reasonable. Unfortunately, I'm not able to read the code/propose changes before a week.

Feel free to keep working on this meanwhile, if there is anything else!

CoffeeStraw avatar Jun 24 '21 22:06 CoffeeStraw

Hey, I'm again with my laptop and I've reviewed your PR. There are some interesting ideas you've implemented to better organize the code, but overall I think the implementation could be done better.

Here's what I propose:

  • Rename "input" function in "set_input" (and so "output" in "set_output");
  • Rename "load" function in "parse_ass";
  • Remove "add_pyonfx_extension" and move it back in "parse_ass", I don't like it to be separated nor I find it usefull;
  • Delete "validate" function. Also, as mentioned in issue #29, we should follow EAFP approach, so we should avoid all the checks whenever possible;
  • "Untitled.ass" when no input is specified does not work. Instead, we should hardcode an empty .ass and use it as input when no one is specified. I can see the use-case of this option, as one could want to just generate new lines;
  • I should investigate whether it is really not possible to extract any information from lines if no video is specified, I'm not totally sure. Thank you anyway for pointing out this issue!

I hope I've written everything I thought. Feel free to point out other changes, I will also contribute to your branch to speed up the process (but this could take some weeks as I'm busy now).

CoffeeStraw avatar Jul 06 '21 18:07 CoffeeStraw

Here's what I propose:

Rename "input" function in "set_input" (and so "output" in "set_output"); Rename "load" function in "parse_ass";

Accepted.

Remove "add_pyonfx_extension" and move it back in "parse_ass", I don't like it to be separated nor I find it usefull;

As just ass creation without karaoke, the extension is not needed, so i kept it

Delete "validate" function. Also, as mentioned in issue

ok i integrated the test

LBYL -> EAFP #29, we should follow EAFP approach, so we should avoid all the checks whenever possible;

It's your religion or politic , I choose FREEDOM ;p

"Untitled.ass" when no input is specified does not work. Instead, we should hardcode an empty .ass and use it >as input when no one is specified.

This is the purpose of Untitled.ass, matching the spirit of handling it in lua with aegisub ; yes it was buggy, now it is fixed

I can see the use-case of this option, as one could want to just generate new lines;

Here is a common usage of it with an exporter:

# coding:utf8

# messages2ass.py,   dictionary to subtitle exporter to automate translation
# author sosie-js
# require my pythonfx mofied version adding fixes and del_line facility
#==========================================

# For relative imports to work
import os, sys; sys.path.append(os.path.dirname(os.path.realpath(__file__)))
from pyonfx import *

# will include message_en.py a dictiatary of English messages for the syncplay gui
# https://raw.githubusercontent.com/Syncplay/syncplay/master/syncplay/messages_en.py
import messages_en

dict_message_file="message_en" #.py
ass_message_file=dict_message_file+".ass"
print("Welcome on the %s dictionary to ass file %s exporter" % (dict_message_file,ass_message_file) )
print("-------------------------------------------------------------------------------------")
 
io = Ass() #Will use aegisub template Untitled.ass as basis instead of "in.ass"
io.set_output(ass_message_file)
meta, styles, lines = io.get_data()

#messages will hold the message dict
messages=messages_en.en
i=len(lines)
pos_time=0

#the fist line of Untitled.ass, empty, will serve as template
line= lines[0].copy()
duration=2000

for key, value in messages.items():
    print("Exporting value of key %s as subtiyle line" % key)
    l= line.copy()
    i=i+1
    l.i=i
    l.start_time = pos_time
    l.end_time = pos_time+duration
    l.text =value
    io.write_line(l)
    pos_time=pos_time+duration

#Don't forget to remove the pollution lines of the template 
# in our case remove the empty single line of Untitled.ass.
io.del_line(1)   

io.save()
io.open_aegisub()

Download: messages2ass.py.txt

I should investigate whether it is really not possible to extract any information from lines if no video is specified, >I'm not totally sure. Thank you anyway for pointing out this issue!

I explained why on the line, if (not hasattr(self.meta,"play_res_x") or not hasattr(self.meta,"play_res_y")): it depends on playres_x ans playres_y available only in the ass file if video was provided This the case thus when dummy video is choosen

I hope I've written everything I thought. Feel free to point out other changes, I will also contribute to your branch >to speed up the process (but this could take some weeks as I'm busy now).

Bunny Busy too ;p

sosie-js avatar Aug 12 '21 15:08 sosie-js