From 82f661cc256ecd88aefe8db67a05f7557f5c08a8 Mon Sep 17 00:00:00 2001
From: Rob Watson <rob@netflux.io>
Date: Wed, 18 Apr 2018 09:09:24 +0200
Subject: [PATCH] Minor cleanup and fixes

- Small refactor to remove a function body
- Check parameter type in Frame.from_header/1
- Prefer doctest to mpeg_audio_frame_parser_test
---
 lib/mpeg_audio_frame_parser/frame.ex  |  3 +-
 lib/mpeg_audio_frame_parser/impl.ex   | 16 +++-------
 test/mpeg_audio_frame_parser_test.exs | 45 ---------------------------
 3 files changed, 6 insertions(+), 58 deletions(-)
 delete mode 100644 test/mpeg_audio_frame_parser_test.exs

diff --git a/lib/mpeg_audio_frame_parser/frame.ex b/lib/mpeg_audio_frame_parser/frame.ex
index e274181..8b798ac 100644
--- a/lib/mpeg_audio_frame_parser/frame.ex
+++ b/lib/mpeg_audio_frame_parser/frame.ex
@@ -17,7 +17,8 @@ defmodule MPEGAudioFrameParser.Frame do
   @header_length 32
 
   def from_header(header)
-  when bit_size(header) == @header_length
+  when is_binary(header)
+  and bit_size(header) == @header_length
   do
     frame = %Frame{data: header}
     |> Map.put(:version_id, parse_version(header))
diff --git a/lib/mpeg_audio_frame_parser/impl.ex b/lib/mpeg_audio_frame_parser/impl.ex
index 7e9272b..dc8f808 100644
--- a/lib/mpeg_audio_frame_parser/impl.ex
+++ b/lib/mpeg_audio_frame_parser/impl.ex
@@ -62,19 +62,11 @@ defmodule MPEGAudioFrameParser.Impl do
     process_bytes(%{state | current_frame: nil}, rest)
   end
 
-  # Synced, frame complete, and looks like another sync word. Create a new frame struct:
-  defp process_bytes(%{current_frame: %Frame{complete: true}} = state, <<@sync_word::size(11), header::size(21), rest::bits>>) do
-    header = <<@sync_word::size(11), header::size(21)>>
-    frames = [state.current_frame | state.frames]
-    new_state = %{state | current_frame: Frame.from_header(header), frames: frames}
-    process_bytes(new_state, rest)
-  end
-
-  # Synced, frame complete, but next frame looks bad. Prepend, discard a byte and unsync:
+  # Synced, and the current frame is complete:
   defp process_bytes(%{current_frame: %Frame{complete: true}} = state, packet) do
-    Logger.warn "Lost sync. Discarding a byte and searching again for a sync word."
-    <<_byte, rest>> = state.current_frame.data
-    process_bytes(%{state | current_frame: nil}, <<rest, packet::bits>>)
+    frames = [state.current_frame | state.frames]
+    new_state = %{state | current_frame: nil, frames: frames}
+    process_bytes(new_state, packet)
   end
 
   # Synced, current frame not complete and we have bytes available. Add bytes to frame:
diff --git a/test/mpeg_audio_frame_parser_test.exs b/test/mpeg_audio_frame_parser_test.exs
deleted file mode 100644
index 58251d3..0000000
--- a/test/mpeg_audio_frame_parser_test.exs
+++ /dev/null
@@ -1,45 +0,0 @@
-defmodule MPEGAudioFrameParserTest do
-  use ExUnit.Case
-  alias MPEGAudioFrameParser.Frame
-  doctest MPEGAudioFrameParser
-
-  # MP3, 128kbps, no CRC protection, 44100hz, no padding, stereo
-  @frame1 <<0b11111111111_11_01_0_1001_00_0_0_00_00_0_0_00::size(32), 1::size(3304)>>
-  @frame2 <<0b11111111111_11_01_0_1001_00_0_0_00_00_0_0_00::size(32), 0::size(3304)>>
-
-  test "start_link" do
-    MPEGAudioFrameParser.start_link()
-  end
-
-  test "add_packet" do
-    MPEGAudioFrameParser.start_link()
-    MPEGAudioFrameParser.add_packet(@frame1)
-    result = MPEGAudioFrameParser.add_packet(@frame2)
-
-    assert [%Frame{data: @frame1}] = result
-  end
-
-  test "cast_packet" do
-    MPEGAudioFrameParser.start_link()
-    MPEGAudioFrameParser.cast_packet(@frame1)
-    MPEGAudioFrameParser.cast_packet(@frame2)
-  end
-
-  test "pop_frame" do
-    MPEGAudioFrameParser.start_link()
-    MPEGAudioFrameParser.cast_packet(@frame1)
-    MPEGAudioFrameParser.cast_packet(@frame2)
-
-    assert %Frame{data: @frame1} = MPEGAudioFrameParser.pop_frame()
-    assert nil == MPEGAudioFrameParser.pop_frame()
-  end
-
-  test "flush" do
-    MPEGAudioFrameParser.start_link()
-    MPEGAudioFrameParser.cast_packet(@frame1)
-    MPEGAudioFrameParser.cast_packet(@frame2)
-    MPEGAudioFrameParser.flush()
-
-    assert nil == MPEGAudioFrameParser.pop_frame()
-  end
-end