emu: Fixed issues with memory views and snapshot naming.

Include device tag in memory view save item registration names. Without this, instantiating two of the same device (or two devices with identically named memory views) causes a fatal error on start. Fixed -aviwrite/-mngwrite with -snapview native more than two screens. Previously it would attempt to use the same file name for all screens but the first. Improved naming of snapshots, especially when using -snapview native with -aviwrite/-mngwrite (see GitHub #10005). The automatically included screen number should come before the extension if supplied. Also, assume that users actually know what they’re doing if they include a dot in a snapshot filename pattern.
mamedev · Jul 1, 2022 · 875ee33 · 875ee33
1 parent 08e728c
commit 875ee33
Show file tree

Hide file tree

Showing 2 changed files with 69 additions and 29 deletions.
diff --git a/src/emu/emumem_mview.cpp b/src/emu/emumem_mview.cpp
@@ -645,8 +645,8 @@ memory_view::~memory_view()
 
 void memory_view::register_state()
 {
-	m_device.machine().save().save_item(&m_device, "view", m_name.c_str(), 0, NAME(m_cur_slot));
-	m_device.machine().save().save_item(&m_device, "view", m_name.c_str(), 0, NAME(m_cur_id));
+	m_device.machine().save().save_item(&m_device, "view", m_device.subtag(m_name).c_str(), 0, NAME(m_cur_slot));
+	m_device.machine().save().save_item(&m_device, "view", m_device.subtag(m_name).c_str(), 0, NAME(m_cur_id));
 	m_device.machine().save().register_postload(save_prepost_delegate(NAME([this]() { m_handler_read->select_a(m_cur_id); m_handler_write->select_a(m_cur_id); })));
 }
 

diff --git a/src/emu/video.cpp b/src/emu/video.cpp
@@ -380,18 +380,15 @@ void video_manager::save_active_screen_snapshots()
 
 void video_manager::begin_recording_screen(const std::string &filename, uint32_t index, screen_device *screen, movie_recording::format format)
 {
-	// determine the file extension
-	const char *extension = movie_recording::format_file_extension(format);
-
 	// create the emu_file
-	bool is_absolute_path = !filename.empty() && osd_is_absolute_path(filename);
+	bool const is_absolute_path = !filename.empty() && osd_is_absolute_path(filename);
 	std::unique_ptr<emu_file> movie_file = std::make_unique<emu_file>(
 			is_absolute_path ? "" : machine().options().snapshot_directory(),
 			OPEN_FLAG_WRITE | OPEN_FLAG_CREATE | OPEN_FLAG_CREATE_PATHS);
 
 	// and open the actual file
 	std::error_condition filerr = filename.empty()
-			? open_next(*movie_file, extension)
+			? open_next(*movie_file, movie_recording::format_file_extension(format))
 			: movie_file->open(filename);
 	if (filerr)
 	{
@@ -429,26 +426,48 @@ void video_manager::begin_recording(const char *name, movie_recording::format fo
 	// clear out existing recordings
 	m_movie_recordings.clear();
 
+	// check if the supplied name already has the desired extension
+	std::string_view basename;
+	std::string extension;
+	if (name)
+	{
+		std::string_view const desired_ext = movie_recording::format_file_extension(format);
+		basename = name;
+		extension.reserve(1 + desired_ext.length());
+		extension.assign(1, '.').append(desired_ext);
+		if (core_filename_ends_with(basename, extension))
+		{
+			extension = basename.substr(basename.length() - extension.length());
+			basename.remove_suffix(extension.length());
+		}
+	}
+
 	if (m_snap_native)
 	{
+		std::string tempname;
 		for (uint32_t index = 0; index < count; index++, iter++)
 		{
-			create_snapshot_bitmap(iter.current());
+			create_snapshot_bitmap(iter.current()); // TODO: only do this when starting on-the-fly, and make name match AVI file name
 
-			std::string tempname;
 			if (name)
-				tempname = index > 0 ? name : util::string_format("%s%d", name, index);
+			{
+				if (1 < count)
+					tempname = util::string_format("%s%d%s", basename, index, extension);
+				else
+					tempname.assign(name).append(extension);
+			}
+
 			begin_recording_screen(
-				tempname,
-				index,
-				iter.current(),
-				format);
+					tempname,
+					index,
+					iter.current(),
+					format);
 		}
 	}
 	else
 	{
 		create_snapshot_bitmap(nullptr);
-		begin_recording_screen(name ? name : "", 0, iter.current(), format);
+		begin_recording_screen(std::string(basename) + extension, 0, iter.current(), format);
 	}
 }
 
@@ -967,10 +986,23 @@ void video_manager::recompute_speed(const attotime &emutime)
 	if (m_seconds_to_run != 0 && emutime.seconds() >= m_seconds_to_run)
 	{
 		// create a final screenshot
-		emu_file file(machine().options().snapshot_directory(), OPEN_FLAG_WRITE | OPEN_FLAG_CREATE | OPEN_FLAG_CREATE_PATHS);
-		std::error_condition const filerr = open_next(file, "png");
-		if (!filerr)
-			save_snapshot(nullptr, file);
+		if (m_snap_native)
+		{
+			for (screen_device &screen : screen_device_enumerator(machine().root_device()))
+			{
+				emu_file file(machine().options().snapshot_directory(), OPEN_FLAG_WRITE | OPEN_FLAG_CREATE | OPEN_FLAG_CREATE_PATHS);
+				std::error_condition const filerr = open_next(file, "png");
+				if (!filerr)
+					save_snapshot(&screen, file);
+			}
+		}
+		else
+		{
+			emu_file file(machine().options().snapshot_directory(), OPEN_FLAG_WRITE | OPEN_FLAG_CREATE | OPEN_FLAG_CREATE_PATHS);
+			std::error_condition const filerr = open_next(file, "png");
+			if (!filerr)
+				save_snapshot(nullptr, file);
+		}
 
 		//printf("Scheduled exit at %f\n", emutime.as_double());
 		// schedule our demise
@@ -1064,14 +1096,19 @@ std::error_condition video_manager::open_next(emu_file &file, const char *extens
 	// handle defaults
 	const char *snapname = machine().options().snap_name();
 
-	if (snapname == nullptr || snapname[0] == 0)
+	if (!snapname || !snapname[0])
 		snapname = "%g/%i";
 	std::string snapstr(snapname);
 
-	// strip any extension in the provided name
-	int index = snapstr.find_last_of('.');
-	if (index != -1)
-		snapstr = snapstr.substr(0, index);
+	// strip desired extension if already present
+	std::string extstr;
+	extstr.reserve(1 + std::strlen(extension));
+	extstr.assign(1, '.').append(extension);
+	if (core_filename_ends_with(snapstr, extstr))
+	{
+		extstr = snapstr.substr(snapstr.length() - extstr.length());
+		snapstr.resize(snapstr.length() - extstr.length());
+	}
 
 	// handle %d in the template (for image devices)
 	std::string snapdev("%d_");
@@ -1142,21 +1179,24 @@ std::error_condition video_manager::open_next(emu_file &file, const char *extens
 		strreplace(snapstr, "%t", t_str);
 	}
 
-	// add our own extension
-	snapstr.append(".").append(extension);
+	// append extension
+	snapstr.append(extstr);
 
 	// substitute path and gamename up front
 	strreplace(snapstr, "/", PATH_SEPARATOR);
 	strreplace(snapstr, "%g", machine().basename());
 
-	// determine if the template has an index; if not, we always use the same name
+	// determine if the template has an index
 	std::string fname;
 	if (snapstr.find("%i") == -1)
+	{
+		// if not, we always use the same name
 		fname.assign(snapstr);
-
-	// otherwise, we scan for the next available filename
+	}
 	else
 	{
+		// otherwise, we scan for the next available filename
+
 		// try until we succeed
 		file.set_openflags(OPEN_FLAG_WRITE);
 		for (int seq = 0; ; seq++)