trjhtr

I'm writing a media player in WPF and since movies are playable, subtitles are a must.

Here's what it looks like so far:

On the left is the settings tab, in the middle is the actual player and on the right is the playlist.

I feel like asking 1 big question wont be as beneficial as breaking it down into 3 questions, where each one covers a specific aspect of the system. This specific one is the most fundamental - reading and storing the subtitle segments' information.

Part 2

There are a lot of supporting classes involved and while it would be nice to get them reviewed as well, I'd like to put the main focus on the subtitle related classes.

I started by creating the subtitle model classes. Subtitles have a starting/end point and some content. The first characteristic seems like something I might need to use in the future so I decided to write an interface for it:

public interface IInterval<T> : IEquatable<T>, IComparable<T>
 where T : IInterval<T>

 TimeSpan Start get; 
 TimeSpan End get; 

And later inherited by the concrete SubtitleInterval:

[Serializable]
public class SubtitleInterval : IInterval<SubtitleInterval>

 public TimeSpan Start get; 
 public TimeSpan End get; 

 public TimeSpan Duration => End.Subtract(Start);

 public SubtitleInterval(TimeSpan start, TimeSpan end)
 
 Start = start;
 End = end;
 

 public override string ToString()
 
 return $"Start --> End";
 

 #region Implementation of IEquatable<SubtitleInterval>

 public bool Equals(SubtitleInterval other)
 
 if (ReferenceEquals(null, other)) return false;
 if (ReferenceEquals(this, other)) return true;
 return Start.Equals(other.Start) && End.Equals(other.End);
 

 public override bool Equals(object obj)
 
 if (ReferenceEquals(null, obj)) return false;
 if (ReferenceEquals(this, obj)) return true;
 if (obj.GetType() != this.GetType()) return false;
 return Equals((SubtitleInterval)obj);
 

 public override int GetHashCode()
 
 unchecked
 
 return (Start.GetHashCode() * 397) ^ End.GetHashCode();
 
 

 #endregion

 #region Implementation of IComparable<SubtitleInterval>

 public int CompareTo(SubtitleInterval other)
 
 if (ReferenceEquals(this, other)) return 0;
 if (ReferenceEquals(null, other)) return 1;
 var startComparison = Start.CompareTo(other.Start);
 if (startComparison != 0) return startComparison;
 return End.CompareTo(other.End);
 

 #endregion

Next is the actual Model for the subtitles, it consists mainly of 2 properties - Interval and Content, IEquatable<> and IComparable<> are implemented as well:

[Serializable]
public class SubtitleSegment : IEquatable<SubtitleSegment>, IComparable<SubtitleSegment>

 public SubtitleInterval Interval get; 

 public string Content get; 

 public SubtitleSegment([NotNull] SubtitleInterval subtitleInterval, string content)
 
 Interval = subtitleInterval ?? throw new ArgumentNullException(nameof(subtitleInterval));
 Content = content;
 

 public override string ToString()
 
 return $"Interval Environment.NewLine Content";
 

 #region IEquatable implementation

 public bool Equals(SubtitleSegment other)
 
 if (ReferenceEquals(null, other)) return false;
 if (ReferenceEquals(this, other)) return true;
 return Equals(Interval, other.Interval) && string.Equals(Content, other.Content);
 

 public override bool Equals(object obj)
 
 if (ReferenceEquals(null, obj)) return false;
 if (ReferenceEquals(this, obj)) return true;
 if (obj.GetType() != this.GetType()) return false;
 return Equals((SubtitleSegment)obj);
 

 public override int GetHashCode()
 
 unchecked
 
 return ((Interval != null ? Interval.GetHashCode() : 0) * 397) ^
 (Content != null ? Content.GetHashCode() : 0);
 
 

 #endregion

 #region IComparable implementation

 public int CompareTo(SubtitleSegment other)
 
 if (ReferenceEquals(this, other)) return 0;
 if (ReferenceEquals(null, other)) return 1;
 return Interval.CompareTo(Interval);
 

 #endregion

I also wanted .srt files that share the same name with the movie, located in the current played movie's directory or any sub-directory, to be automatically played, instead of manually inserting them.

I also have a setting changeable by the user, which allows for preferred language of the automatically detected subtitles to be set. This is usually indicated by a suffix in the file's name e.g: MovieName.en.srt, MovieName.bg.srt.. In case there is no file with the corresponding suffix the first one that doesn't have any will be selected.

For that purpose I added the SubtitleDetector static class:

public static class SubtitleDetector

 public static FileInformation DetectSubtitles(
 [NotNull] MediaFileInformation file,
 string preferedSubtitleLanguage)
 
 if (file == null) throw new ArgumentNullException(nameof(file));

 var availableSubtitles =
 file.FileInfo.Directory.GetFiles($"*Settings.SubtitleExtensionString", SearchOption.AllDirectories);

 if (!string.IsNullOrEmpty(preferedSubtitleLanguage))
 
 if (preferedSubtitleLanguage[0] != '.')
 
 preferedSubtitleLanguage = preferedSubtitleLanguage.Insert(0, ".");
 

 var preferedLanguageSubtitle = availableSubtitles
 .Where(s => s.Name.Contains(
 $"preferedSubtitleLanguageSettings.SubtitleExtensionString"))
 .FirstOrDefault(info => Path.GetFileNameWithoutExtension(info.Name) ==
 $"file.FileNamepreferedSubtitleLanguage");

 if (preferedLanguageSubtitle != null)
 
 return new FileInformation(preferedLanguageSubtitle.FullName);
 
 

 return availableSubtitles.Where(subs => Path.GetFileNameWithoutExtension(subs.Name) == file.FileName)
 .Select(subs => new FileInformation(subs.FullName)).FirstOrDefault();
 

Next I needed some way to read the actual content of the .srt file, first I started by inspecting the way they are written and luckily the format was rather simple:

Start --> End
Content

Start --> End
Content

00:00:00,012 --> 00:00:02,244
Content1

00:00:09:368 --> 00:00:12,538
Content2

There are some extra rules to where exactly you can put :, , or . when indicating the interval of the subtitles, but I wont dig too much into that.

public sealed class SubtitleReader

 public Encoding Encoding get; 

 public SubtitleReader([NotNull] Encoding encoding)
 
 Encoding = encoding ?? throw new ArgumentNullException(nameof(encoding));
 

 public CircularList<SubtitleSegment> ExtractSubtitles([NotNull] string path)
 
 if (string.IsNullOrEmpty(path)) throw new ArgumentNullException(nameof(path));

 var subtitles = new CircularList<SubtitleSegment>();
 using (var sr = new StreamReader(path, Encoding))
 
 var text = sr.ReadToEnd();
 var lines = text.Split(new "rn" , StringSplitOptions.None);
 for (int i = 0; i < lines.Length; i++)
 
 if (TryParseSubtitleInterval(lines[i], out var interval))
 
 var content = ExtractCurrentSubtitleContent(i, lines);
 subtitles.Add(new SubtitleSegment(interval, content));
 
 
 
 return subtitles.OrderBy(s => s).ToCircularList();
 

 private string ExtractCurrentSubtitleContent(int startIndex, string lines)
 
 var subtitleContent = new StringBuilder();
 int endIndex = Array.IndexOf(lines, string.Empty, startIndex);
 for (int i = startIndex + 1; i < endIndex; i++)
 
 subtitleContent.AppendLine(lines[i].Trim(' '));
 
 return subtitleContent.ToString();
 

 private bool TryParseSubtitleInterval(string input, out SubtitleInterval interval)
 
 interval = null;
 if (string.IsNullOrEmpty(input))
 
 return false;
 
 var segments = input.Split(new Settings.SubtitleSeparationString , StringSplitOptions.None);
 if (segments.Length != 2)
 
 return false;
 
 segments = segments.Select(s => s.Trim(' ').Replace(',', '.').Replace('.', ':')).ToArray();
 if (TimeSpan.TryParseExact(segments[0], Settings.GetTimeSpanStringFormats(), DateTimeFormatInfo.InvariantInfo,
 out var start) &&
 TimeSpan.TryParseExact(segments[1], Settings.GetTimeSpanStringFormats(), DateTimeFormatInfo.InvariantInfo,
 out var end) &&
 start < end)
 
 interval = new SubtitleInterval(start, end);
 return true;
 
 return false;
 

Where the supplied TimeSpanFormats are as follows:

private static readonly string _timeSpanStringFormats =

 @"h:m:s",
 @"h:m:s:f",
 @"h:m:s:ff",
 @"h:m:s:fff",
 @"h:m:ss",
 @"h:m:ss:f",
 @"h:m:ss:ff",
 @"h:m:ss:fff",
 @"h:mm:s",
 @"h:mm:s:f",
 @"h:mm:s:ff",
 @"h:mm:s:fff",
 @"h:mm:ss",
 @"h:mm:ss:f",
 @"h:mm:ss:ff",
 @"h:mm:ss:fff",
 @"hh:m:s",
 @"hh:m:s:f",
 @"hh:m:s:ff",
 @"hh:m:s:fff",
 @"hh:m:ss",
 @"hh:m:ss:f",
 @"hh:m:ss:ff",
 @"hh:m:ss:fff",
 @"hh:mm:s",
 @"hh:mm:s:f",
 @"hh:mm:s:ff",
 @"hh:mm:s:fff",
 @"hh:mm:ss",
 @"hh:mm:ss:f",
 @"hh:mm:ss:ff",
 @"hh:mm:ss:fff",
;

And the CircularList<> implementation:

public interface ICircularList<T> : IList<T>

 T Next get; 
 T Previous get; 
 T MoveNext();
 T MovePrevious();
 T Current get; 
 void SetCurrent(int currentIndex);
 void Reset();


public class CircularList<T> : ICircularList<T>

 private readonly IList<T> _elements = new List<T>();

 private int _lastUsedElementIndex;

 public CircularList(IEnumerable<T> collection, int startingIterableIndex = 0)
 
 foreach (T item in collection)
 
 _elements.Add(item);
 
 _lastUsedElementIndex = startingIterableIndex;
 

 public CircularList()
 
 

 #region Implementation of IEnumerable

 public IEnumerator<T> GetEnumerator()
 
 return _elements.GetEnumerator();
 

 IEnumerator IEnumerable.GetEnumerator()
 
 return GetEnumerator();
 

 #endregion

 #region Implementation of ICollection<T>

 public void Add(T item)
 
 _elements.Add(item);
 

 public void Clear()
 
 _elements.Clear();
 

 public bool Contains(T item)
 
 return _elements.Contains(item);
 

 public void CopyTo(T array, int arrayIndex)
 
 _elements.CopyTo(array, arrayIndex);
 

 public bool Remove(T item)
 
 return _elements.Remove(item);
 

 public int Count => _elements.Count;

 public bool IsReadOnly => false;

 #endregion

 #region Implementation of IList<T>
 public int IndexOf(T item)
 
 return _elements.IndexOf(item);
 

 public void Insert(int index, T item)
 
 _elements.Insert(index, item);
 

 public void RemoveAt(int index)
 
 _elements.RemoveAt(index);
 

 public T this[int index]
 
 get => _elements[index];
 set => _elements[index] = value;
 

 #endregion

 #region Implementation of ICircularList<T>

 public T Next => _lastUsedElementIndex + 1 >= _elements.Count
 ? _elements[0]
 : _elements[_lastUsedElementIndex + 1];

 public T Previous => _lastUsedElementIndex - 1 < 0
 ? _elements[_elements.Count - 1]
 : _elements[_lastUsedElementIndex - 1];

 public T MoveNext()
 
 int temp = _lastUsedElementIndex;
 _lastUsedElementIndex++;
 if (_lastUsedElementIndex >= _elements.Count)
 
 _lastUsedElementIndex = 0;
 
 return _elements[temp];
 

 public T MovePrevious()
 
 int temp = _lastUsedElementIndex;
 _lastUsedElementIndex--;
 if (_lastUsedElementIndex < 0)
 
 _lastUsedElementIndex = _elements.Count - 1;
 
 return _elements[temp];
 

 public T Current => _elements.Count == 0
 ? default(T)
 : _elements[_lastUsedElementIndex];

 public void SetCurrent(int currentIndex)
 
 _lastUsedElementIndex = currentIndex;
 

 public void Reset()
 
 _lastUsedElementIndex = 0;
 

 #endregion

FileInformation classes:

public interface IFileInformation

 string FileName get; 
 FileInfo FileInfo get; 
 Uri Uri get; 


public class FileInformation : IFileInformation, IEquatable<FileInformation>

 public string FileName get; 
 public FileInfo FileInfo get; 
 public Uri Uri get; 

 public FileInformation([NotNull] string filePath)
 : this(new Uri(filePath))
 
 

 public FileInformation([NotNull] Uri fileUri)
 
 Uri = fileUri ?? throw new ArgumentNullException(nameof(fileUri));
 FileInfo = new FileInfo(fileUri.OriginalString);
 FileName = Path.GetFileNameWithoutExtension(FileInfo.Name);
 

 #region Equality members

 public bool Equals(FileInformation other)
 
 if (ReferenceEquals(null, other)) return false;
 if (ReferenceEquals(this, other)) return true;
 return string.Equals(FileName, other.FileName) && Equals(FileInfo, other.FileInfo) && Equals(Uri, other.Uri);
 

 public override bool Equals(object obj)
 
 if (ReferenceEquals(null, obj)) return false;
 if (ReferenceEquals(this, obj)) return true;
 if (obj.GetType() != this.GetType()) return false;
 return Equals((FileInformation)obj);
 

 public override int GetHashCode()
 
 unchecked
 
 var hashCode = (FileName != null ? FileName.GetHashCode() : 0);
 hashCode = (hashCode * 397) ^ (FileInfo != null ? FileInfo.GetHashCode() : 0);
 hashCode = (hashCode * 397) ^ (Uri != null ? Uri.GetHashCode() : 0);
 return hashCode;
 
 

 #endregion

public class MediaFileInformation : DependencyObject, IFileInformation, INotifyPropertyChanged, IEquatable<MediaFileInformation>

 public TimeSpan FileLength get; 
 public string FileName get; 
 public FileInfo FileInfo get; 
 public Uri Uri get; 

 public static readonly DependencyProperty IsPlayingProperty =
 DependencyProperty.Register(nameof(IsPlaying), typeof(bool), typeof(MediaFileInformation),
 new PropertyMetadata(null));

 public bool IsPlaying
 
 get => (bool)GetValue(IsPlayingProperty);
 set
 
 SetValue(IsPlayingProperty, value);
 OnPropertyChanged();
 
 

 public MediaFileInformation([NotNull] string filePath)
 : this(new Uri(filePath))
 
 

 public MediaFileInformation([NotNull] Uri fileUri)
 
 Uri = fileUri ?? throw new ArgumentNullException(nameof(fileUri));
 FileInfo = new FileInfo(fileUri.OriginalString);
 FileName = Path.GetFileNameWithoutExtension(FileInfo.Name);
 FileLength = FileInfo.GetFileDuration();
 

 #region INotifyPropertyChanged Implementation

 public event PropertyChangedEventHandler PropertyChanged;

 protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null)
 
 PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
 

 #endregion

 #region Equality members

 public bool Equals(MediaFileInformation other)
 
 if (ReferenceEquals(null, other)) return false;
 if (ReferenceEquals(this, other)) return true;
 return FileLength.Equals(other.FileLength) && string.Equals(FileName, other.FileName) &&
 Equals(FileInfo, other.FileInfo) && Equals(Uri, other.Uri);
 

 #endregion

What I think can be improved...

The bool Equals(object obj) method of the SubtitleInterval does not need to repeat the implementation of its strongly typed counterpart. You could use the new is operator and redirect it like this:

return obj is SubtitleInterval si && Equals(si);

You can do the same with the SubtitleSegment and FileInformation classes.

Instead of the old ?:

Interval != null ? Interval.GetHashCode() : 0

you can now use a combination of the new ? and ?? and make it simpler:

Interval?.GetHashCode() ?? 0

You seem to like negative conditions...

if (!string.IsNullOrEmpty(preferedSubtitleLanguage))

and

if (preferedSubtitleLanguage[0] != '.')

and

if (preferedLanguageSubtitle != null)

I find that positive ones are easier to understand so I suggest trying to flip them where possible and use early returns that would also contribute to less nesting.

if (preferedSubtitleLanguage[0] != '.')

This conditios is too magical. You should introduce a helper variable, and/or use a const explaining the '.', and/or use a const for the 0 index explaining its purpose.

if (TimeSpan.TryParseExact(segments[0], Settings.GetTimeSpanStringFormats(), DateTimeFormatInfo.InvariantInfo,
 out var start) &&
 TimeSpan.TryParseExact(segments[1], Settings.GetTimeSpanStringFormats(), DateTimeFormatInfo.InvariantInfo,
 out var end) &&
 start < end)

The poor if :-( I wouldn't put so much code in there, it gets ugly. A new helper method would be cleaner. In fact, the SubtitleInterval could implement a TryParse method.

Did you really write all the _timeSpanStringFormats by hand? I'm lazy, I'd write some code to generate it :-)

public CircularList(IEnumerable<T> collection, int startingIterableIndex = 0)

 foreach (T item in collection)
 
 _elements.Add(item);
 
 _lastUsedElementIndex = startingIterableIndex;

Throw a way the foreach. List<T> has a constructor that takes a collection.

I would try to come up with a better name for the FileInformation type. With something more domain related. Maybe SubtitleFile etc. There is already a FileInfo and creating another, similar type, makes it confusing.

What else I think...

Other than these couple of nitpicks this code is very well structured and pretty clean. Good job!