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

[3.x] Complete refactoring #64

Open
delphiki opened this issue Dec 19, 2017 · 4 comments
Open

[3.x] Complete refactoring #64

delphiki opened this issue Dec 19, 2017 · 4 comments

Comments

@delphiki
Copy link
Member

delphiki commented Dec 19, 2017

The current structure of the lib tends to be bit of a mess.
I'm currently working on a complete refactoring.

The idea is the make the lib comply to the SOLID principles.

The new structure would be something like the following:

src/Captioning/
    Dumper/
        # classes dedicated to dumping (formatting the output)
        SubripDumper
        WebVttDumper
        ...
    Format/
        # classes dedicated to the different formats
        Subrip
        WebVtt
        ...
    Exception/
        # dedicated exception
        ParseException
        DumpException
        CueOverlapException
        ...
    Parser/
        # classes dedicated to parsing
        SubripParser
        WebVttParser
        ...
tests/

Of course this would also imply a new way to use the lib, for example, to parse/dump a Subrip file:

$subripFile = Subrip::parseFile('my-sub.srt'); // returns an instance of SubripFile

// with options:
$subripFile = Subrip::parseFile('my-sub.srt', ['strict' => false, /* ... */]); // returns an instance of SubripFile

Subrip::dump($subripFile); // returns a formatted string (instead of actually writing the file)

// with options
Subrip::dump($subripFile, ['stripTags' => true]);

This would allow to separate the encoding/decoding handler into a dedicated class.

This is still a WIP, and any feedback would be appreciated!

@delphiki delphiki changed the title [4.x] Complete refactoring [3.x] Complete refactoring Dec 19, 2017
@skrivy
Copy link

skrivy commented Mar 12, 2018

Feel free to setup basic skeleton and we can contribute.

However, in my opinion, I would try to avoid the static calls. Factory pattern should be used to create an instance, instead.

Dumper also doesn't sound the best - Formatter sounds better.

@miranovy
Copy link

miranovy commented Mar 13, 2018

We have been thinking with @skrivy about refactoring the library and we have this proposal:

/**
 * General class that contains all required data.
 */
class Subtitle {
  protected $segments = []; // array of subtitle segments
  protected $property = []; // property information (exist better solution?)

  public function add(ISubtitleSegment $segment) { $this->segments[] = $segment; }
  public function remove(int $index) { ... }
  public function getAll() { return $this->segments; }
  
  public function setProperty($key, $value) { $this->property[$key] = $value; }
  public function getProperty($key) { return $this->property[$key]; }
  
  /**
   * Store subtites to string in specific format.
   * @param $formatter Subtitle formatter
   */
  public function format(ISubtitleFormatter $formatter) { 
    return $formatter->format($segments, $property);
  }
 
  public function sort() { ...}
  public function changeFPS(float $old_fps, float $new_fps) { ... }
  public function mergeSegments(array $segments) { ...}
}

/**
 * Subtitle segment interface.
 */
interface ISubtitleSegment {
    public function __construct($startTime, $stopTime, $text);
    public function getStartTime(); // Return start time in miliseconds
    public function getStopTime();  // Return stop time in miliseconds
    public function getText();      // Return subtitle text
}

/**
 * Subtitle segment abstract class.
 */
abstract class AbstractSubtitleSegment implements ISubtitleSegment {
    protected $startTime; // start time in miliseconds
    protected $stopTime;  // stop time in miliseconds
    protected $text;
    
    public function __construct($startTime, $stopTime, $text) { ... }
    public function getStartTime() { return $this->startTime; } 
    public function getStopTime() { return $this->stopTime; }
    public function getText() { return $this->text; }
}

/**
 * Subrip subtitle segment
 */
class SubripSegment extends AbstractSubtitleSegment {
    // Additional methods here if required
}

/**
 * Subtitle format parser interface.
 */ 
interface ISubtitleParser {
    /**
      * @param $options Parser options
      */
    public function __construct($options = []);
    
    /**
      * @param $input Textual representation of subtitle content
      * @return Subtitle|false
      */
    public function parse(string $input);
}

/**
 * Subrip format parser.
 */
 class SubripParser implements ISubtitleParser {
    // ...
}

/**
 * Subtitle formatter interface.
 */
interface ISubtitleFormatter {
    /**
      * @param $factory Segment convertor factory    
      * @param $input Formatter options
      */
    public function __construct($factory, $options = []);
    
    public function format($segments, $property);
}

/**
 * Abstract subtitlte formatter class.
 */
abstract function AbstractSubtitleFormatter implements ISubtitleFormatter {
    protected $factory;

    public function __construct($factory, $options = []) {
        $this->factory = $factory;
    }

    public function format($segments, $property) {
        return $this->getHeader($segments, $property) .
               $this->getBody($segments, $property) . 
               $this->getFooter($segments, $property);
    }

    /**
     * Return name of subtitle segment class.
     * @retrun string
     */
    abstract public function getSegmentClassName();

    abstract protected function getHeader($segments, $property);
    abstract protected function getFooter($segments, $property);
    abstract protected function getSegment(ISubtitleSegment $segment);
    
    protected function getBody($segments, $property) {
        $str = "";   
        foreach ($segments as $segment) {
            $segmentClass = $this->getSegmentClassName();
            $convertor = $factory->create(get_class($segment), $segmentClass);
            $str .= $this->getSegment($convertor === FALSE ? new $segmentClass($segment->getStartTime(), $segment->getStopTime(), $segment->getText()) : $convertor->convert($segment));
        }
        return $str;
    }    
}
 
/**
 * Subrip format formatter.
 */
class SubripFormatter extends AbstractSubtitleFormatter {
    public function getSegmentClassName() { return 'SubripSegment'; }
    protected function getHeader($segments, $property) { return ""; }
    protected function getFooter($segments, $property) { return ""; }
    protected function getSegment(ISubtitleSegment $segment) { ... }
}

/**
 * Segment convertor factory.
 */
class SegmentConvertorFactory
{
    /** @var array Convertor cache */
    private convertorCache = [];

    public function create($from, $to)
    {
        return $this->getInstance($this->getClassName($from, $to));
    }
    
    /**
     * Return class name of converter
     * @return string
     */
    protected function getClassName($from, $to) {
        return preg_replace('/Segment$/', '', $from) . 'To' . $to . 'SegmentConverter';
    }
    
    /**
      * Get instance of subtitle segment convertor.
      * @return ISegmentConvertor
      */
    protected getInstance($name) {
        if (isset($this->convertorCache[$name])) {
            return $this->convertorCache[$name]; // return cached convertor
        } 
        
        if (!class_exists($class)) {
            return FALSE;
        }

        return $this->convertorCache[$name] = new $name;
    }
}

/**
 * Segment convertor inteface.
 */
interface ISegmentConvertor {
    /**
      * Convert segment from one format to another format.
      * @return ISubtitleSegment
      */
    public function convert(ISubtitleSegment $segment);
}

/**
 * Subrip to WebVtt segment convertor.
 */
class SubripToWebVttSegmentConvertor implements ISegmentConvertor {
    /**
      * Convert segment from Subrip format to WebVtt format.
      * @return ISubtitleSegment
      */
   public function convert(ISubtitleSegment $segment) { ... } 
}

Directory structure:

src/Captioning/
    Subrip/
        SubripParser.php
        SubripSegment.php
        SubripFormatter.php
        SubripToWebVttSegmentConvertor.php
    WebVtt/
        WebVttParser.php
        WebVttSegment.php
        WebVttFormatter.php
        WebVttToSubripSegmentConvertor.php
    AbstractSubtitleFormatter.php
    AbstractSubtitleSegment.php
    ISegmentConvertor.php
    ISubtitleParser.php
    ISubtitleSegment.php
    ISubtitleFormatter.php
    SegmentConvertorFactory.php
    Subtitle.php

Examle of reading subtitles from Subrip format and saving to WebVtt format:

    // create subtitles from string in Subrip format
    $srt = new SubripParser($options);
    $subtitles = $srt->parse($string);
    
    // store subtites to string in WebVtt format
    $str = $subtitles->format(new WebVttFormatter(new SegmentConvertorFactory(), $options));

@maidmaid
Copy link

About the I*.php interface files, why not suffix interfaces with Interface like Symfony does.

ISegmentConvertor.php -> SegmentConvertorInterface.php

@skrivy
Copy link

skrivy commented Aug 16, 2018

@maidmaid It's just matter of chosen coding standard. As this is standalone library, we are open to everything.

@delphiki delphiki pinned this issue Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants