Merge lp:~jamesh/mediascanner2/bug-1436110 into lp:mediascanner2

[James Henstridge]

Treat invalid dates in MP3s as missing metadata.

If an MP3 includes a year of "-1", or the date is otherwise unparsable, treat that metadata as missing.

I couldn't trigger the bug mentioned in the bug using vorbis files, so I've added tests using mp3 sample files. These tests should only run when the corresponding codec is available, so won't cover us in Jenkins (since the non-free codecs aren't available), but will catch problems when run locally.

I also took the opportunity to add a test for standard metadata extraction on an MP3 file.

[lp:~jamesh/mediascanner2/bug-1436110]

301. By James Henstridge on 2015-06-03

Move QML plugin directory so that it isn't shadowed by the installed
version.

302. By James Henstridge on 2015-06-03

Update test to actually check what it claims to, and work now that we've
got more test audio files.

303. By James Henstridge on 2015-06-04

More cleanups.

[Michi Henning (michihenning)]

LGTM!

review: Approve

=== modified file 'CMakeLists.txt'
--- CMakeLists.txt	2014-10-09 08:48:05 +0000
+++ CMakeLists.txt	2015-06-04 00:53:07 +0000
@@ -41,7 +41,7 @@
 add_subdirectory(src/mediascanner)
 add_subdirectory(src/daemon)
 add_subdirectory(src/ms-dbus)
-add_subdirectory(src/qml/Ubuntu/MediaScanner)
+add_subdirectory(src/qml/Ubuntu/MediaScanner.0.1)
 add_subdirectory(src/utils)
 add_subdirectory(test)
 

=== modified file 'src/daemon/MetadataExtractor.cc'
--- src/daemon/MetadataExtractor.cc	2015-01-22 09:04:17 +0000
+++ src/daemon/MetadataExtractor.cc	2015-06-04 00:53:07 +0000
@@ -172,6 +172,9 @@
         } else if (G_VALUE_HOLDS(val, GST_TYPE_DATE_TIME)) {
             if (tagname == GST_TAG_DATE_TIME) {
                 GstDateTime *dt = static_cast<GstDateTime*>(g_value_get_boxed(val));
+                if (!dt) {
+                    continue;
+                }
                 char *dt_string = gst_date_time_to_iso8601_string(dt);
                 mfb->setDate(dt_string);
                 g_free(dt_string);

=== renamed directory 'src/qml/Ubuntu/MediaScanner' => 'src/qml/Ubuntu/MediaScanner.0.1'
=== modified file 'test/basic.cc'
--- test/basic.cc	2014-10-24 13:55:20 +0000
+++ test/basic.cc	2015-06-04 00:53:07 +0000
@@ -224,10 +224,16 @@
 
 TEST(Mediascanner, root_skip) {
     MetadataExtractor e;
-    string root(SOURCE_DIR);
+    string root(SOURCE_DIR "/media");
     Scanner s(&e, root, AudioMedia);
-    s.next();
-    ASSERT_THROW(s.next(), StopIteration);
+    while (true) {
+        try {
+            auto d = s.next();
+            EXPECT_EQ(std::string::npos, d.filename.find("fake_root")) << d.filename;
+        } catch (const StopIteration &e) {
+            break;
+        }
+    }
 }
 
 TEST(Mediascanner, scan_files_found_in_new_dir) {

=== added file 'test/media/baddate.mp3'
Binary files test/media/baddate.mp3	1970-01-01 00:00:00 +0000 and test/media/baddate.mp3	2015-06-04 00:53:07 +0000 differ
=== added file 'test/media/baddate.ogg'
Binary files test/media/baddate.ogg	1970-01-01 00:00:00 +0000 and test/media/baddate.ogg	2015-06-04 00:53:07 +0000 differ
=== added file 'test/media/testfile.mp3'
Binary files test/media/testfile.mp3	1970-01-01 00:00:00 +0000 and test/media/testfile.mp3	2015-06-04 00:53:07 +0000 differ
=== modified file 'test/test_metadataextractor.cc'
--- test/test_metadataextractor.cc	2015-01-22 09:04:17 +0000
+++ test/test_metadataextractor.cc	2015-06-04 00:53:07 +0000
@@ -25,6 +25,7 @@
 #include<algorithm>
 #include<stdexcept>
 #include<cstdio>
+#include<memory>
 #include<string>
 #include<gst/gst.h>
 #include <gtest/gtest.h>
@@ -32,6 +33,46 @@
 using namespace std;
 using namespace mediascanner;
 
+namespace {
+
+bool supports_decoder(const std::string& format)
+{
+    static std::set<std::string> formats;
+
+    if (formats.empty())
+    {
+        std::unique_ptr<GList, decltype(&gst_plugin_feature_list_free)> decoders(
+            gst_element_factory_list_get_elements(GST_ELEMENT_FACTORY_TYPE_DECODER, GST_RANK_NONE),
+            gst_plugin_feature_list_free);
+        for (const GList* l = decoders.get(); l != nullptr; l = l->next)
+        {
+            const auto factory = static_cast<GstElementFactory*>(l->data);
+
+            const GList* templates = gst_element_factory_get_static_pad_templates(factory);
+            for (const GList* l = templates; l != nullptr; l = l->next)
+            {
+                const auto t = static_cast<GstStaticPadTemplate*>(l->data);
+                if (t->direction != GST_PAD_SINK)
+                {
+                    continue;
+                }
+
+                std::unique_ptr<GstCaps, decltype(&gst_caps_unref)> caps(gst_static_caps_get(&t->static_caps),
+                                                                         gst_caps_unref);
+                for (unsigned int i = 0; i < gst_caps_get_size(caps.get()); i++)
+                {
+                    const auto structure = gst_caps_get_structure(caps.get(), i);
+                    formats.emplace(gst_structure_get_name(structure));
+                }
+            }
+        }
+    }
+
+    return formats.find(format) != formats.end();
+}
+
+}
+
 class MetadataExtractorTest : public ::testing::Test {
  protected:
   MetadataExtractorTest() {
@@ -89,6 +130,25 @@
     EXPECT_EQ(file.getDuration(), 5);
 }
 
+TEST_F(MetadataExtractorTest, extract_mp3) {
+    if (!supports_decoder("audio/mpeg")) {
+        printf("MP3 codec not supported\n");
+        return;
+    }
+    MetadataExtractor e;
+    string testfile = SOURCE_DIR "/media/testfile.mp3";
+    MediaFile file = e.extract(e.detect(testfile));
+
+    EXPECT_EQ(file.getType(), AudioMedia);
+    EXPECT_EQ(file.getTitle(), "track1");
+    EXPECT_EQ(file.getAuthor(), "artist1");
+    EXPECT_EQ(file.getAlbum(), "album1");
+    EXPECT_EQ(file.getDate(), "2013-06-03");
+    EXPECT_EQ(file.getTrackNumber(), 1);
+    EXPECT_EQ(file.getDuration(), 1);
+    EXPECT_EQ(file.getGenre(), "Hip-Hop");
+}
+
 TEST_F(MetadataExtractorTest, extract_video) {
     MetadataExtractor e;
 
@@ -133,17 +193,44 @@
     EXPECT_DOUBLE_EQ(153.1727346, file.getLongitude());
 }
 
+TEST_F(MetadataExtractorTest, extract_bad_date) {
+    MetadataExtractor e;
+    string testfile = SOURCE_DIR "/media/baddate.ogg";
+    MediaFile file = e.extract(e.detect(testfile));
+
+    EXPECT_EQ(file.getType(), AudioMedia);
+    EXPECT_EQ(file.getTitle(), "Track");
+    EXPECT_EQ(file.getAuthor(), "Artist");
+    EXPECT_EQ(file.getAlbum(), "Album");
+    EXPECT_EQ(file.getDate(), "");
+}
+
+TEST_F(MetadataExtractorTest, extract_mp3_bad_date) {
+    if (!supports_decoder("audio/mpeg")) {
+        printf("MP3 codec not supported\n");
+        return;
+    }
+    MetadataExtractor e;
+    string testfile = SOURCE_DIR "/media/baddate.mp3";
+    MediaFile file = e.extract(e.detect(testfile));
+
+    EXPECT_EQ(file.getType(), AudioMedia);
+    EXPECT_EQ(file.getTitle(), "Track");
+    EXPECT_EQ(file.getAuthor(), "Artist");
+    EXPECT_EQ(file.getAlbum(), "Album");
+    EXPECT_EQ(file.getDate(), "");
+}
+
 TEST_F(MetadataExtractorTest, blacklist) {
     MetadataExtractor e;
     string testfile = SOURCE_DIR "/media/playlist.m3u";
     try {
         e.detect(testfile);
+        FAIL();
     } catch(const std::runtime_error &e) {
         std::string error_message(e.what());
         ASSERT_NE(error_message.find("blacklist"), std::string::npos);
-        return;
     }
-    ASSERT_TRUE(false) << "Blacklist exception was not thrown.\n";
 }
 
 TEST_F(MetadataExtractorTest, png_file) {